| Message ID | 1523440025-18077-2-git-send-email-amelie.delaunay@st.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 11, 2018 at 11:47:01AM +0200, Amelie Delaunay wrote: > This patch adds documentation of device tree bindings for the > STMicroelectronics Multi-Function eXpander (STMFX) GPIO expander. > > Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com> > --- > .../devicetree/bindings/pinctrl/pinctrl-stmfx.txt | 118 +++++++++++++++++++++ > 1 file changed, 118 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pinctrl/pinctrl-stmfx.txt > > diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-stmfx.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-stmfx.txt > new file mode 100644 > index 0000000..4d8941de > --- /dev/null > +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-stmfx.txt > @@ -0,0 +1,118 @@ > +STMicroelectronics Multi-Function eXpander (STMFX) GPIO expander bindings > + > +ST Multi-Function eXpander (STMFX) is a slave controller using I2C for > +communication with the main MCU. It offers up to 24 GPIOs expansion. > + > +Required properties: > +- compatible: should be "st,stmfx-0300". > +- reg: I2C slave address of the device. > +- interrupt-parent: phandle of the STMFX parent interrupt controller. > +- interrutps: interrupt specifier triggered by MFX_IRQ_OUT signal. > + > +Optional property: s/property/properties/ > +- drive-open-drain: configure MFX_IRQ_OUT as open drain. > +- vdd-supply: phandle of the regulator supplying STMFX. > + > +Required properties for gpio controller sub-node: > +- #gpio-cells: should be <2>, the first cell is the GPIO number and the second > + cell is the gpio flags in accordance with <dt-bindings/gpio/gpio.h>. > +- gpio-controller: marks the device as a GPIO controller. > +Please refer to ../gpio/gpio.txt. > + > +Optional properties for gpio controller sub-node: > +- #interrupt-cells: should be <2>, the first cell is the GPIO number and the > + second cell is the interrupt flags in accordance with > + <dt-bindings/interrupt-controller/irq.h>. > +- interrupt-controller: marks the device as an interrupt controller. > + > +Please refer to pinctrl-bindings.txt for pin configuration. > + > +Required properties for pin configuration sub-nodes: > +- pins: list of pins to which the configuration applies. > + > +Optional properties for pin configuration sub-nodes (pinconf-generic ones): > +- bias-disable: disable any bias on the pin. > +- bias-pull-up: the pin will be pulled up. > +- bias-pull-pin-default: use the pin-default pull state. > +- bias-pull-down: the pin will be pulled down. > +- drive-open-drain: the pin will be driven with open drain. > +- drive-push-pull: the pin will be driven actively high and low. > +- output-high: the pin will be configured as an output driving high level. > +- output-low: the pin will be configured as an output driving low level. > + > +Note that STMFX pins[15:0] are called "gpio[15:0]", and STMFX pins[23:16] are > +called "agpio[7:0]". Example, to refer to pin 18 of STMFX, use "agpio2". > + > +Example: > + > + stmfxpinctrl: stmfx@42 { > + compatible = "st,stmfx-0300"; > + reg = <0x42>; > + interrupt-parent = <&gpioi>; > + interrupts = <8 IRQ_TYPE_EDGE_RISING>; > + vdd-supply = <&v3v3>; > + status = "okay"; Don't show status in examples. > + > + stmfxgpio: gpio { Why does this need to be a sub node? Are there functions beyond GPIO? > + #gpio-cells = <2>; > + #interrupt-cells = <2>; > + gpio-controller; > + interrupt-controller; > + status = "okay"; > + }; > + > + joystick_pins: joystick@0 { A unit-address without a reg prop is not valid. > + pins = "gpio0", "gpio1", "gpio2", "gpio3", "gpio4"; > + drive-push-pull; > + bias-pull-down; > + }; > + }; > + > + joystick { > + compatible = "gpio-keys"; > + #address-cells = <1>; > + #size-cells = <0>; > + pinctrl-0 = <&joystick_pins>; > + pinctrl-names = "default"; > + button@0 { > + label = "JoySel"; > + linux,code = <KEY_ENTER>; > + interrupt-parent = <&stmfxgpio>; > + interrupts = <0 IRQ_TYPE_EDGE_RISING>; > + }; > + button@1 { > + label = "JoyDown"; > + linux,code = <KEY_DOWN>; > + interrupt-parent = <&stmfxgpio>; > + interrupts = <1 IRQ_TYPE_EDGE_RISING>; > + }; > + button@2 { > + label = "JoyLeft"; > + linux,code = <KEY_LEFT>; > + interrupt-parent = <&stmfxgpio>; > + interrupts = <2 IRQ_TYPE_EDGE_RISING>; > + }; > + button@3 { > + label = "JoyRight"; > + linux,code = <KEY_RIGHT>; > + interrupt-parent = <&stmfxgpio>; > + interrupts = <3 IRQ_TYPE_EDGE_RISING>; > + }; > + button@4 { > + label = "JoyUp"; > + linux,code = <KEY_UP>; > + interrupt-parent = <&stmfxgpio>; > + interrupts = <4 IRQ_TYPE_EDGE_RISING>; > + }; > + }; > + > + leds { > + compatible = "gpio-leds"; > + orange { > + gpios = <&stmfxgpio 17 1>; > + }; > + > + blue { > + gpios = <&stmfxgpio 19 1>; > + }; > + } > -- > 2.7.4 >
On Mon, Apr 16, 2018 at 8:19 PM, Rob Herring <robh@kernel.org> wrote: > On Wed, Apr 11, 2018 at 11:47:01AM +0200, Amelie Delaunay wrote: >> This patch adds documentation of device tree bindings for the >> STMicroelectronics Multi-Function eXpander (STMFX) GPIO expander. >> >> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com> >> + stmfxpinctrl: stmfx@42 { >> + compatible = "st,stmfx-0300"; >> + reg = <0x42>; >> + interrupt-parent = <&gpioi>; >> + interrupts = <8 IRQ_TYPE_EDGE_RISING>; >> + vdd-supply = <&v3v3>; >> + status = "okay"; > > Don't show status in examples. > >> + >> + stmfxgpio: gpio { > > Why does this need to be a sub node? Are there functions beyond GPIO? Amelie can answer to whether there are, I suspect there are and in the review of patch 2 I suggest a MFD parent and parent/child spawning of a MFD child for the GPIO and pin control device. Yours, Linus Walleij
On 04/16/2018 08:19 PM, Rob Herring wrote: > On Wed, Apr 11, 2018 at 11:47:01AM +0200, Amelie Delaunay wrote: >> This patch adds documentation of device tree bindings for the >> STMicroelectronics Multi-Function eXpander (STMFX) GPIO expander. >> >> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com> >> --- >> .../devicetree/bindings/pinctrl/pinctrl-stmfx.txt | 118 +++++++++++++++++++++ >> 1 file changed, 118 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/pinctrl/pinctrl-stmfx.txt >> >> diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-stmfx.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-stmfx.txt >> new file mode 100644 >> index 0000000..4d8941de >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-stmfx.txt >> @@ -0,0 +1,118 @@ >> +STMicroelectronics Multi-Function eXpander (STMFX) GPIO expander bindings >> + >> +ST Multi-Function eXpander (STMFX) is a slave controller using I2C for >> +communication with the main MCU. It offers up to 24 GPIOs expansion. >> + >> +Required properties: >> +- compatible: should be "st,stmfx-0300". >> +- reg: I2C slave address of the device. >> +- interrupt-parent: phandle of the STMFX parent interrupt controller. >> +- interrutps: interrupt specifier triggered by MFX_IRQ_OUT signal. >> + >> +Optional property: > > s/property/properties/ > >> +- drive-open-drain: configure MFX_IRQ_OUT as open drain. >> +- vdd-supply: phandle of the regulator supplying STMFX. >> + >> +Required properties for gpio controller sub-node: >> +- #gpio-cells: should be <2>, the first cell is the GPIO number and the second >> + cell is the gpio flags in accordance with <dt-bindings/gpio/gpio.h>. >> +- gpio-controller: marks the device as a GPIO controller. >> +Please refer to ../gpio/gpio.txt. >> + >> +Optional properties for gpio controller sub-node: >> +- #interrupt-cells: should be <2>, the first cell is the GPIO number and the >> + second cell is the interrupt flags in accordance with >> + <dt-bindings/interrupt-controller/irq.h>. >> +- interrupt-controller: marks the device as an interrupt controller. >> + >> +Please refer to pinctrl-bindings.txt for pin configuration. >> + >> +Required properties for pin configuration sub-nodes: >> +- pins: list of pins to which the configuration applies. >> + >> +Optional properties for pin configuration sub-nodes (pinconf-generic ones): >> +- bias-disable: disable any bias on the pin. >> +- bias-pull-up: the pin will be pulled up. >> +- bias-pull-pin-default: use the pin-default pull state. >> +- bias-pull-down: the pin will be pulled down. >> +- drive-open-drain: the pin will be driven with open drain. >> +- drive-push-pull: the pin will be driven actively high and low. >> +- output-high: the pin will be configured as an output driving high level. >> +- output-low: the pin will be configured as an output driving low level. >> + >> +Note that STMFX pins[15:0] are called "gpio[15:0]", and STMFX pins[23:16] are >> +called "agpio[7:0]". Example, to refer to pin 18 of STMFX, use "agpio2". >> + >> +Example: >> + >> + stmfxpinctrl: stmfx@42 { >> + compatible = "st,stmfx-0300"; >> + reg = <0x42>; >> + interrupt-parent = <&gpioi>; >> + interrupts = <8 IRQ_TYPE_EDGE_RISING>; >> + vdd-supply = <&v3v3>; >> + status = "okay"; > > Don't show status in examples. > I'll fix it. >> + >> + stmfxgpio: gpio { > > Why does this need to be a sub node? Are there functions beyond GPIO? > I'll address this point in my response to Linus. >> + #gpio-cells = <2>; >> + #interrupt-cells = <2>; >> + gpio-controller; >> + interrupt-controller; >> + status = "okay"; >> + }; >> + >> + joystick_pins: joystick@0 { > > A unit-address without a reg prop is not valid. > OK, I'll replace it by 'joystick_pins: joystick-0'. >> + pins = "gpio0", "gpio1", "gpio2", "gpio3", "gpio4"; >> + drive-push-pull; >> + bias-pull-down; >> + }; >> + }; >> + >> + joystick { >> + compatible = "gpio-keys"; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + pinctrl-0 = <&joystick_pins>; >> + pinctrl-names = "default"; >> + button@0 { >> + label = "JoySel"; >> + linux,code = <KEY_ENTER>; >> + interrupt-parent = <&stmfxgpio>; >> + interrupts = <0 IRQ_TYPE_EDGE_RISING>; >> + }; >> + button@1 { >> + label = "JoyDown"; >> + linux,code = <KEY_DOWN>; >> + interrupt-parent = <&stmfxgpio>; >> + interrupts = <1 IRQ_TYPE_EDGE_RISING>; >> + }; >> + button@2 { >> + label = "JoyLeft"; >> + linux,code = <KEY_LEFT>; >> + interrupt-parent = <&stmfxgpio>; >> + interrupts = <2 IRQ_TYPE_EDGE_RISING>; >> + }; >> + button@3 { >> + label = "JoyRight"; >> + linux,code = <KEY_RIGHT>; >> + interrupt-parent = <&stmfxgpio>; >> + interrupts = <3 IRQ_TYPE_EDGE_RISING>; >> + }; >> + button@4 { >> + label = "JoyUp"; >> + linux,code = <KEY_UP>; >> + interrupt-parent = <&stmfxgpio>; >> + interrupts = <4 IRQ_TYPE_EDGE_RISING>; >> + }; >> + }; >> + >> + leds { >> + compatible = "gpio-leds"; >> + orange { >> + gpios = <&stmfxgpio 17 1>; >> + }; >> + >> + blue { >> + gpios = <&stmfxgpio 19 1>; >> + }; >> + } >> -- >> 2.7.4 >> Thanks, Amelie
On 04/26/2018 02:49 PM, Linus Walleij wrote: > On Mon, Apr 16, 2018 at 8:19 PM, Rob Herring <robh@kernel.org> wrote: >> On Wed, Apr 11, 2018 at 11:47:01AM +0200, Amelie Delaunay wrote: >>> This patch adds documentation of device tree bindings for the >>> STMicroelectronics Multi-Function eXpander (STMFX) GPIO expander. >>> >>> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com> > >>> + stmfxpinctrl: stmfx@42 { >>> + compatible = "st,stmfx-0300"; >>> + reg = <0x42>; >>> + interrupt-parent = <&gpioi>; >>> + interrupts = <8 IRQ_TYPE_EDGE_RISING>; >>> + vdd-supply = <&v3v3>; >>> + status = "okay"; >> >> Don't show status in examples. >> >>> + >>> + stmfxgpio: gpio { >> >> Why does this need to be a sub node? Are there functions beyond GPIO? > > Amelie can answer to whether there are, I suspect there > are and in the review of patch 2 I suggest a MFD parent > and parent/child spawning of a MFD child for the GPIO > and pin control device. > > Yours, > Linus Walleij > Indeed, stmfx has other functions than GPIO. But, after comments done here: [1] and there: [2], it has been decided to move MFD parent/GPIO child drivers into a single PINCTRL/GPIO driver because of the following reasons: - Other stmfx functions (IDD measurement and TouchScreen controller) are not used on any of the boards using an stmfx and supported by Linux, so no way to test these functions, and no need to maintain them while they are not being used. - But, in the case a new board will use more than GPIO function on stmfx, the actual implementation allow to easily extract common init part of stmfx and put it in an MFD driver. So I could remove gpio sub-node and put its contents in stmfx node and keep single PINCTRL/GPIO driver for the time being. Please advise, Thanks, Amelie [1] https://lkml.org/lkml/2018/2/12/265 [2] https://lkml.org/lkml/2018/2/21/1503
On Wed, May 9, 2018 at 9:56 AM, Amelie DELAUNAY <amelie.delaunay@st.com> wrote: > Indeed, stmfx has other functions than GPIO. But, after comments done > here: [1] and there: [2], it has been decided to move MFD parent/GPIO > child drivers into a single PINCTRL/GPIO driver because of the following > reasons: > - Other stmfx functions (IDD measurement and TouchScreen controller) are > not used on any of the boards using an stmfx and supported by Linux, so > no way to test these functions, and no need to maintain them while they > are not being used. > - But, in the case a new board will use more than GPIO function on > stmfx, the actual implementation allow to easily extract common init > part of stmfx and put it in an MFD driver. > > So I could remove gpio sub-node and put its contents in stmfx node and > keep single PINCTRL/GPIO driver for the time being. > Please advise, I would normally advice to use the right modeling from the start, create the MFD driver and spawn the devices from there. It is confusing if the layout of the driver(s) doesn't really match the layout of the hardware. I understand that it is a pain to write new MFD drivers to get your things going and it would be "nice to get this working really quick now" but in my experience it is better to do it right from the start. Yours, Linus Walleij
On 05/16/2018 04:20 PM, Linus Walleij wrote: > On Wed, May 9, 2018 at 9:56 AM, Amelie DELAUNAY <amelie.delaunay@st.com> wrote: > >> Indeed, stmfx has other functions than GPIO. But, after comments done >> here: [1] and there: [2], it has been decided to move MFD parent/GPIO >> child drivers into a single PINCTRL/GPIO driver because of the following >> reasons: >> - Other stmfx functions (IDD measurement and TouchScreen controller) are >> not used on any of the boards using an stmfx and supported by Linux, so >> no way to test these functions, and no need to maintain them while they >> are not being used. >> - But, in the case a new board will use more than GPIO function on >> stmfx, the actual implementation allow to easily extract common init >> part of stmfx and put it in an MFD driver. >> >> So I could remove gpio sub-node and put its contents in stmfx node and >> keep single PINCTRL/GPIO driver for the time being. >> Please advise, > > I would normally advice to use the right modeling from the start, create > the MFD driver and spawn the devices from there. It is confusing > if the layout of the driver(s) doesn't really match the layout of the > hardware. > > I understand that it is a pain to write new MFD drivers to get your > things going and it would be "nice to get this working really quick > now" but in my experience it is better to do it right from the start. > Hi Linus, Thanks for your advice. I understand the point. So, the right modeling would be to: - create an MFD driver with the common init part of stmfx - remove all common init part of stmfx-pinctrl driver and keep only all gpio/pinctrl functions. I will not develop the other stmfx functions (IDD measurement driver and TouchScreen controller driver) because, as explained ealier, they are not used on any of the boards using an stmfx and supported by Linux, so no way to test these functions, and no need to maintain them while they are not being used. Lee, are you OK with that ? Regards, Amelie
On Wed, 16 May 2018, Amelie DELAUNAY wrote: > > > On 05/16/2018 04:20 PM, Linus Walleij wrote: > > On Wed, May 9, 2018 at 9:56 AM, Amelie DELAUNAY <amelie.delaunay@st.com> wrote: > > > >> Indeed, stmfx has other functions than GPIO. But, after comments done > >> here: [1] and there: [2], it has been decided to move MFD parent/GPIO > >> child drivers into a single PINCTRL/GPIO driver because of the following > >> reasons: > >> - Other stmfx functions (IDD measurement and TouchScreen controller) are > >> not used on any of the boards using an stmfx and supported by Linux, so > >> no way to test these functions, and no need to maintain them while they > >> are not being used. > >> - But, in the case a new board will use more than GPIO function on > >> stmfx, the actual implementation allow to easily extract common init > >> part of stmfx and put it in an MFD driver. > >> > >> So I could remove gpio sub-node and put its contents in stmfx node and > >> keep single PINCTRL/GPIO driver for the time being. > >> Please advise, > > > > I would normally advice to use the right modeling from the start, create > > the MFD driver and spawn the devices from there. It is confusing > > if the layout of the driver(s) doesn't really match the layout of the > > hardware. > > > > I understand that it is a pain to write new MFD drivers to get your > > things going and it would be "nice to get this working really quick > > now" but in my experience it is better to do it right from the start. > > > > Hi Linus, > > Thanks for your advice. I understand the point. > So, the right modeling would be to: > - create an MFD driver with the common init part of stmfx > - remove all common init part of stmfx-pinctrl driver and keep only all > gpio/pinctrl functions. > > I will not develop the other stmfx functions (IDD measurement driver and > TouchScreen controller driver) because, as explained ealier, they are > not used on any of the boards using an stmfx and supported by Linux, so > no way to test these functions, and no need to maintain them while they > are not being used. > > Lee, are you OK with that ? I missed a lot of this conversation I think, but from what I've read, it sounds fine.
On 05/17/2018 08:36 AM, Lee Jones wrote: > On Wed, 16 May 2018, Amelie DELAUNAY wrote: > >> >> >> On 05/16/2018 04:20 PM, Linus Walleij wrote: >>> On Wed, May 9, 2018 at 9:56 AM, Amelie DELAUNAY <amelie.delaunay@st.com> wrote: >>> >>>> Indeed, stmfx has other functions than GPIO. But, after comments done >>>> here: [1] and there: [2], it has been decided to move MFD parent/GPIO >>>> child drivers into a single PINCTRL/GPIO driver because of the following >>>> reasons: >>>> - Other stmfx functions (IDD measurement and TouchScreen controller) are >>>> not used on any of the boards using an stmfx and supported by Linux, so >>>> no way to test these functions, and no need to maintain them while they >>>> are not being used. >>>> - But, in the case a new board will use more than GPIO function on >>>> stmfx, the actual implementation allow to easily extract common init >>>> part of stmfx and put it in an MFD driver. >>>> >>>> So I could remove gpio sub-node and put its contents in stmfx node and >>>> keep single PINCTRL/GPIO driver for the time being. >>>> Please advise, >>> >>> I would normally advice to use the right modeling from the start, create >>> the MFD driver and spawn the devices from there. It is confusing >>> if the layout of the driver(s) doesn't really match the layout of the >>> hardware. >>> >>> I understand that it is a pain to write new MFD drivers to get your >>> things going and it would be "nice to get this working really quick >>> now" but in my experience it is better to do it right from the start. >>> >> >> Hi Linus, >> >> Thanks for your advice. I understand the point. >> So, the right modeling would be to: >> - create an MFD driver with the common init part of stmfx >> - remove all common init part of stmfx-pinctrl driver and keep only all >> gpio/pinctrl functions. >> >> I will not develop the other stmfx functions (IDD measurement driver and >> TouchScreen controller driver) because, as explained ealier, they are >> not used on any of the boards using an stmfx and supported by Linux, so >> no way to test these functions, and no need to maintain them while they >> are not being used. >> >> Lee, are you OK with that ? > > I missed a lot of this conversation I think, but from what I've read, > it sounds fine. > I summarize the situation: - I still don't have an official datasheet for STMFX device which could justify the use of an MFD driver; - the MFD driver will contain the STMFX chip initialization stuff such as regmap initialization (regmap structure will be shared with the child), chip initialization, global interrupt management; - there will be only one child (GPIO/PINCTRL node) for the time being. So, is "MFD driver + GPIO/PINCTRL driver" the right modeling, and does it still sound fine after this summary ? :) Thanks, Amelie
On Fri, 18 May 2018, Amelie DELAUNAY wrote: > On 05/17/2018 08:36 AM, Lee Jones wrote: > > On Wed, 16 May 2018, Amelie DELAUNAY wrote: > > > >> > >> > >> On 05/16/2018 04:20 PM, Linus Walleij wrote: > >>> On Wed, May 9, 2018 at 9:56 AM, Amelie DELAUNAY <amelie.delaunay@st.com> wrote: > >>> > >>>> Indeed, stmfx has other functions than GPIO. But, after comments done > >>>> here: [1] and there: [2], it has been decided to move MFD parent/GPIO > >>>> child drivers into a single PINCTRL/GPIO driver because of the following > >>>> reasons: > >>>> - Other stmfx functions (IDD measurement and TouchScreen controller) are > >>>> not used on any of the boards using an stmfx and supported by Linux, so > >>>> no way to test these functions, and no need to maintain them while they > >>>> are not being used. > >>>> - But, in the case a new board will use more than GPIO function on > >>>> stmfx, the actual implementation allow to easily extract common init > >>>> part of stmfx and put it in an MFD driver. > >>>> > >>>> So I could remove gpio sub-node and put its contents in stmfx node and > >>>> keep single PINCTRL/GPIO driver for the time being. > >>>> Please advise, > >>> > >>> I would normally advice to use the right modeling from the start, create > >>> the MFD driver and spawn the devices from there. It is confusing > >>> if the layout of the driver(s) doesn't really match the layout of the > >>> hardware. > >>> > >>> I understand that it is a pain to write new MFD drivers to get your > >>> things going and it would be "nice to get this working really quick > >>> now" but in my experience it is better to do it right from the start. > >>> > >> > >> Hi Linus, > >> > >> Thanks for your advice. I understand the point. > >> So, the right modeling would be to: > >> - create an MFD driver with the common init part of stmfx > >> - remove all common init part of stmfx-pinctrl driver and keep only all > >> gpio/pinctrl functions. > >> > >> I will not develop the other stmfx functions (IDD measurement driver and > >> TouchScreen controller driver) because, as explained ealier, they are > >> not used on any of the boards using an stmfx and supported by Linux, so > >> no way to test these functions, and no need to maintain them while they > >> are not being used. > >> > >> Lee, are you OK with that ? > > > > I missed a lot of this conversation I think, but from what I've read, > > it sounds fine. > > > > I summarize the situation: > - I still don't have an official datasheet for STMFX device which could > justify the use of an MFD driver; > - the MFD driver will contain the STMFX chip initialization stuff such > as regmap initialization (regmap structure will be shared with the > child), chip initialization, global interrupt management; > - there will be only one child (GPIO/PINCTRL node) for the time being. > > So, is "MFD driver + GPIO/PINCTRL driver" the right modeling, and does > it still sound fine after this summary ? :) It is starting to sound like there will only ever be one child device, which starts to cross the line into "this is not an MFD" (M = Multi) territory.
On 05/18/2018 03:52 PM, Lee Jones wrote: > On Fri, 18 May 2018, Amelie DELAUNAY wrote: > >> On 05/17/2018 08:36 AM, Lee Jones wrote: >>> On Wed, 16 May 2018, Amelie DELAUNAY wrote: >>> >>>> >>>> >>>> On 05/16/2018 04:20 PM, Linus Walleij wrote: >>>>> On Wed, May 9, 2018 at 9:56 AM, Amelie DELAUNAY <amelie.delaunay@st.com> wrote: >>>>> >>>>>> Indeed, stmfx has other functions than GPIO. But, after comments done >>>>>> here: [1] and there: [2], it has been decided to move MFD parent/GPIO >>>>>> child drivers into a single PINCTRL/GPIO driver because of the following >>>>>> reasons: >>>>>> - Other stmfx functions (IDD measurement and TouchScreen controller) are >>>>>> not used on any of the boards using an stmfx and supported by Linux, so >>>>>> no way to test these functions, and no need to maintain them while they >>>>>> are not being used. >>>>>> - But, in the case a new board will use more than GPIO function on >>>>>> stmfx, the actual implementation allow to easily extract common init >>>>>> part of stmfx and put it in an MFD driver. >>>>>> >>>>>> So I could remove gpio sub-node and put its contents in stmfx node and >>>>>> keep single PINCTRL/GPIO driver for the time being. >>>>>> Please advise, >>>>> >>>>> I would normally advice to use the right modeling from the start, create >>>>> the MFD driver and spawn the devices from there. It is confusing >>>>> if the layout of the driver(s) doesn't really match the layout of the >>>>> hardware. >>>>> >>>>> I understand that it is a pain to write new MFD drivers to get your >>>>> things going and it would be "nice to get this working really quick >>>>> now" but in my experience it is better to do it right from the start. >>>>> >>>> >>>> Hi Linus, >>>> >>>> Thanks for your advice. I understand the point. >>>> So, the right modeling would be to: >>>> - create an MFD driver with the common init part of stmfx >>>> - remove all common init part of stmfx-pinctrl driver and keep only all >>>> gpio/pinctrl functions. >>>> >>>> I will not develop the other stmfx functions (IDD measurement driver and >>>> TouchScreen controller driver) because, as explained ealier, they are >>>> not used on any of the boards using an stmfx and supported by Linux, so >>>> no way to test these functions, and no need to maintain them while they >>>> are not being used. >>>> >>>> Lee, are you OK with that ? >>> >>> I missed a lot of this conversation I think, but from what I've read, >>> it sounds fine. >>> >> >> I summarize the situation: >> - I still don't have an official datasheet for STMFX device which could >> justify the use of an MFD driver; >> - the MFD driver will contain the STMFX chip initialization stuff such >> as regmap initialization (regmap structure will be shared with the >> child), chip initialization, global interrupt management; >> - there will be only one child (GPIO/PINCTRL node) for the time being. >> >> So, is "MFD driver + GPIO/PINCTRL driver" the right modeling, and does >> it still sound fine after this summary ? :) > > It is starting to sound like there will only ever be one child device, > which starts to cross the line into "this is not an MFD" (M = Multi) > territory. > ... for the time being. So, Linus, Lee, is it possible to find common ground ?
On Fri, May 18, 2018 at 9:29 AM, Amelie DELAUNAY <amelie.delaunay@st.com> wrote: > On 05/17/2018 08:36 AM, Lee Jones wrote: >> On Wed, 16 May 2018, Amelie DELAUNAY wrote: >>> On 05/16/2018 04:20 PM, Linus Walleij wrote: >>>> On Wed, May 9, 2018 at 9:56 AM, Amelie DELAUNAY <amelie.delaunay@st.com> wrote: >>>> >>>>> Indeed, stmfx has other functions than GPIO. But, after comments done >>>>> here: [1] and there: [2], it has been decided to move MFD parent/GPIO >>>>> child drivers into a single PINCTRL/GPIO driver because of the following >>>>> reasons: >>>>> - Other stmfx functions (IDD measurement and TouchScreen controller) are >>>>> not used on any of the boards using an stmfx and supported by Linux, so >>>>> no way to test these functions, and no need to maintain them while they >>>>> are not being used. >>>>> - But, in the case a new board will use more than GPIO function on >>>>> stmfx, the actual implementation allow to easily extract common init >>>>> part of stmfx and put it in an MFD driver. >>>>> >>>>> So I could remove gpio sub-node and put its contents in stmfx node and >>>>> keep single PINCTRL/GPIO driver for the time being. >>>>> Please advise, >>>> >>>> I would normally advice to use the right modeling from the start, create >>>> the MFD driver and spawn the devices from there. It is confusing >>>> if the layout of the driver(s) doesn't really match the layout of the >>>> hardware. >>>> >>>> I understand that it is a pain to write new MFD drivers to get your >>>> things going and it would be "nice to get this working really quick >>>> now" but in my experience it is better to do it right from the start. >>>> >>> >>> Hi Linus, >>> >>> Thanks for your advice. I understand the point. >>> So, the right modeling would be to: >>> - create an MFD driver with the common init part of stmfx >>> - remove all common init part of stmfx-pinctrl driver and keep only all >>> gpio/pinctrl functions. >>> >>> I will not develop the other stmfx functions (IDD measurement driver and >>> TouchScreen controller driver) because, as explained ealier, they are >>> not used on any of the boards using an stmfx and supported by Linux, so >>> no way to test these functions, and no need to maintain them while they >>> are not being used. >>> >>> Lee, are you OK with that ? >> >> I missed a lot of this conversation I think, but from what I've read, >> it sounds fine. >> > > I summarize the situation: > - I still don't have an official datasheet for STMFX device which could > justify the use of an MFD driver; > - the MFD driver will contain the STMFX chip initialization stuff such > as regmap initialization (regmap structure will be shared with the > child), chip initialization, global interrupt management; > - there will be only one child (GPIO/PINCTRL node) for the time being. But there will be more devices in it. And they will invariably be put to use later, and there will be new versions of the chip as well, and then you will be happy about doing the MFD core, which makes it easy to add new variants with different subdevices. > So, is "MFD driver + GPIO/PINCTRL driver" the right modeling, and does > it still sound fine after this summary ? :) No I think it should use an MFD core. Mainly because of device tree concerns. The main reason is that the device tree bindings will be different if you add an MFD core later, the GPIO and pinctrl driver will move to a child node, making old device trees incompatible. We could have a single driver in GPIO+pin control if it is a child of an MFD node in the device tree, but it doesn't make much sense as the I2C device need to be probing to the MFD core. Yours, Linus Walleij
On 05/24/2018 09:13 AM, Linus Walleij wrote: > On Fri, May 18, 2018 at 9:29 AM, Amelie DELAUNAY <amelie.delaunay@st.com> wrote: >> On 05/17/2018 08:36 AM, Lee Jones wrote: >>> On Wed, 16 May 2018, Amelie DELAUNAY wrote: >>>> On 05/16/2018 04:20 PM, Linus Walleij wrote: >>>>> On Wed, May 9, 2018 at 9:56 AM, Amelie DELAUNAY <amelie.delaunay@st.com> wrote: >>>>> >>>>>> Indeed, stmfx has other functions than GPIO. But, after comments done >>>>>> here: [1] and there: [2], it has been decided to move MFD parent/GPIO >>>>>> child drivers into a single PINCTRL/GPIO driver because of the following >>>>>> reasons: >>>>>> - Other stmfx functions (IDD measurement and TouchScreen controller) are >>>>>> not used on any of the boards using an stmfx and supported by Linux, so >>>>>> no way to test these functions, and no need to maintain them while they >>>>>> are not being used. >>>>>> - But, in the case a new board will use more than GPIO function on >>>>>> stmfx, the actual implementation allow to easily extract common init >>>>>> part of stmfx and put it in an MFD driver. >>>>>> >>>>>> So I could remove gpio sub-node and put its contents in stmfx node and >>>>>> keep single PINCTRL/GPIO driver for the time being. >>>>>> Please advise, >>>>> >>>>> I would normally advice to use the right modeling from the start, create >>>>> the MFD driver and spawn the devices from there. It is confusing >>>>> if the layout of the driver(s) doesn't really match the layout of the >>>>> hardware. >>>>> >>>>> I understand that it is a pain to write new MFD drivers to get your >>>>> things going and it would be "nice to get this working really quick >>>>> now" but in my experience it is better to do it right from the start. >>>>> >>>> >>>> Hi Linus, >>>> >>>> Thanks for your advice. I understand the point. >>>> So, the right modeling would be to: >>>> - create an MFD driver with the common init part of stmfx >>>> - remove all common init part of stmfx-pinctrl driver and keep only all >>>> gpio/pinctrl functions. >>>> >>>> I will not develop the other stmfx functions (IDD measurement driver and >>>> TouchScreen controller driver) because, as explained ealier, they are >>>> not used on any of the boards using an stmfx and supported by Linux, so >>>> no way to test these functions, and no need to maintain them while they >>>> are not being used. >>>> >>>> Lee, are you OK with that ? >>> >>> I missed a lot of this conversation I think, but from what I've read, >>> it sounds fine. >>> >> >> I summarize the situation: >> - I still don't have an official datasheet for STMFX device which could >> justify the use of an MFD driver; >> - the MFD driver will contain the STMFX chip initialization stuff such >> as regmap initialization (regmap structure will be shared with the >> child), chip initialization, global interrupt management; >> - there will be only one child (GPIO/PINCTRL node) for the time being. > > But there will be more devices in it. And they will invariably be put > to use later, and there will be new versions of the chip as well, and > then you will be happy about doing the MFD core, which makes it > easy to add new variants with different subdevices. > >> So, is "MFD driver + GPIO/PINCTRL driver" the right modeling, and does >> it still sound fine after this summary ? :) > > No I think it should use an MFD core. > > Mainly because of device tree concerns. > > The main reason is that the device tree bindings will be different if > you add an MFD core later, the GPIO and pinctrl driver will > move to a child node, making old device trees incompatible. > > We could have a single driver in GPIO+pin control if it is a child > of an MFD node in the device tree, but it doesn't make much > sense as the I2C device need to be probing to the MFD core. > I agree with you Linus, and that's why all STMFX chip initialization stuff was decorrelated in pinctrl-stmfx. This shows that this stuff needs to be in an MFD core. But as there is only one child for now (due to the reasons mentioned earlier), it can suggest that it is not a Multi-Function Device. I'm not able to target when IDD or TS functions will be required on a Linux product, but it still makes sense to consider that these functions will be used on a Linux product. So, I think MFD core + GPIO/pinctrl driver is the right modeling, but I wanted to be sure that this is okay for everyone. I don't want to spend time on something that will not be accepted due to its modeling. Regards, Amelie
On Mon, 28 May 2018, Amelie DELAUNAY wrote: > On 05/24/2018 09:13 AM, Linus Walleij wrote: > > On Fri, May 18, 2018 at 9:29 AM, Amelie DELAUNAY <amelie.delaunay@st.com> wrote: > >> On 05/17/2018 08:36 AM, Lee Jones wrote: > >>> On Wed, 16 May 2018, Amelie DELAUNAY wrote: > >>>> On 05/16/2018 04:20 PM, Linus Walleij wrote: > >>>>> On Wed, May 9, 2018 at 9:56 AM, Amelie DELAUNAY <amelie.delaunay@st.com> wrote: > >>>>> > >>>>>> Indeed, stmfx has other functions than GPIO. But, after comments done > >>>>>> here: [1] and there: [2], it has been decided to move MFD parent/GPIO > >>>>>> child drivers into a single PINCTRL/GPIO driver because of the following > >>>>>> reasons: > >>>>>> - Other stmfx functions (IDD measurement and TouchScreen controller) are > >>>>>> not used on any of the boards using an stmfx and supported by Linux, so > >>>>>> no way to test these functions, and no need to maintain them while they > >>>>>> are not being used. > >>>>>> - But, in the case a new board will use more than GPIO function on > >>>>>> stmfx, the actual implementation allow to easily extract common init > >>>>>> part of stmfx and put it in an MFD driver. > >>>>>> > >>>>>> So I could remove gpio sub-node and put its contents in stmfx node and > >>>>>> keep single PINCTRL/GPIO driver for the time being. > >>>>>> Please advise, > >>>>> > >>>>> I would normally advice to use the right modeling from the start, create > >>>>> the MFD driver and spawn the devices from there. It is confusing > >>>>> if the layout of the driver(s) doesn't really match the layout of the > >>>>> hardware. > >>>>> > >>>>> I understand that it is a pain to write new MFD drivers to get your > >>>>> things going and it would be "nice to get this working really quick > >>>>> now" but in my experience it is better to do it right from the start. > >>>>> > >>>> > >>>> Hi Linus, > >>>> > >>>> Thanks for your advice. I understand the point. > >>>> So, the right modeling would be to: > >>>> - create an MFD driver with the common init part of stmfx > >>>> - remove all common init part of stmfx-pinctrl driver and keep only all > >>>> gpio/pinctrl functions. > >>>> > >>>> I will not develop the other stmfx functions (IDD measurement driver and > >>>> TouchScreen controller driver) because, as explained ealier, they are > >>>> not used on any of the boards using an stmfx and supported by Linux, so > >>>> no way to test these functions, and no need to maintain them while they > >>>> are not being used. > >>>> > >>>> Lee, are you OK with that ? > >>> > >>> I missed a lot of this conversation I think, but from what I've read, > >>> it sounds fine. > >>> > >> > >> I summarize the situation: > >> - I still don't have an official datasheet for STMFX device which could > >> justify the use of an MFD driver; > >> - the MFD driver will contain the STMFX chip initialization stuff such > >> as regmap initialization (regmap structure will be shared with the > >> child), chip initialization, global interrupt management; > >> - there will be only one child (GPIO/PINCTRL node) for the time being. > > > > But there will be more devices in it. And they will invariably be put > > to use later, and there will be new versions of the chip as well, and > > then you will be happy about doing the MFD core, which makes it > > easy to add new variants with different subdevices. > > > >> So, is "MFD driver + GPIO/PINCTRL driver" the right modeling, and does > >> it still sound fine after this summary ? :) > > > > No I think it should use an MFD core. > > > > Mainly because of device tree concerns. > > > > The main reason is that the device tree bindings will be different if > > you add an MFD core later, the GPIO and pinctrl driver will > > move to a child node, making old device trees incompatible. > > > > We could have a single driver in GPIO+pin control if it is a child > > of an MFD node in the device tree, but it doesn't make much > > sense as the I2C device need to be probing to the MFD core. > > > > I agree with you Linus, and that's why all STMFX chip initialization > stuff was decorrelated in pinctrl-stmfx. This shows that this stuff > needs to be in an MFD core. > > But as there is only one child for now (due to the reasons mentioned > earlier), it can suggest that it is not a Multi-Function Device. > > I'm not able to target when IDD or TS functions will be required on a > Linux product, but it still makes sense to consider that these functions > will be used on a Linux product. > > So, I think MFD core + GPIO/pinctrl driver is the right modeling, but I > wanted to be sure that this is okay for everyone. I don't want to spend > time on something that will not be accepted due to its modeling. It's fine. Go ahead. Thanks for seeing this through to a reasonable conclusion.
diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-stmfx.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-stmfx.txt new file mode 100644 index 0000000..4d8941de --- /dev/null +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-stmfx.txt @@ -0,0 +1,118 @@ +STMicroelectronics Multi-Function eXpander (STMFX) GPIO expander bindings + +ST Multi-Function eXpander (STMFX) is a slave controller using I2C for +communication with the main MCU. It offers up to 24 GPIOs expansion. + +Required properties: +- compatible: should be "st,stmfx-0300". +- reg: I2C slave address of the device. +- interrupt-parent: phandle of the STMFX parent interrupt controller. +- interrutps: interrupt specifier triggered by MFX_IRQ_OUT signal. + +Optional property: +- drive-open-drain: configure MFX_IRQ_OUT as open drain. +- vdd-supply: phandle of the regulator supplying STMFX. + +Required properties for gpio controller sub-node: +- #gpio-cells: should be <2>, the first cell is the GPIO number and the second + cell is the gpio flags in accordance with <dt-bindings/gpio/gpio.h>. +- gpio-controller: marks the device as a GPIO controller. +Please refer to ../gpio/gpio.txt. + +Optional properties for gpio controller sub-node: +- #interrupt-cells: should be <2>, the first cell is the GPIO number and the + second cell is the interrupt flags in accordance with + <dt-bindings/interrupt-controller/irq.h>. +- interrupt-controller: marks the device as an interrupt controller. + +Please refer to pinctrl-bindings.txt for pin configuration. + +Required properties for pin configuration sub-nodes: +- pins: list of pins to which the configuration applies. + +Optional properties for pin configuration sub-nodes (pinconf-generic ones): +- bias-disable: disable any bias on the pin. +- bias-pull-up: the pin will be pulled up. +- bias-pull-pin-default: use the pin-default pull state. +- bias-pull-down: the pin will be pulled down. +- drive-open-drain: the pin will be driven with open drain. +- drive-push-pull: the pin will be driven actively high and low. +- output-high: the pin will be configured as an output driving high level. +- output-low: the pin will be configured as an output driving low level. + +Note that STMFX pins[15:0] are called "gpio[15:0]", and STMFX pins[23:16] are +called "agpio[7:0]". Example, to refer to pin 18 of STMFX, use "agpio2". + +Example: + + stmfxpinctrl: stmfx@42 { + compatible = "st,stmfx-0300"; + reg = <0x42>; + interrupt-parent = <&gpioi>; + interrupts = <8 IRQ_TYPE_EDGE_RISING>; + vdd-supply = <&v3v3>; + status = "okay"; + + stmfxgpio: gpio { + #gpio-cells = <2>; + #interrupt-cells = <2>; + gpio-controller; + interrupt-controller; + status = "okay"; + }; + + joystick_pins: joystick@0 { + pins = "gpio0", "gpio1", "gpio2", "gpio3", "gpio4"; + drive-push-pull; + bias-pull-down; + }; + }; + + joystick { + compatible = "gpio-keys"; + #address-cells = <1>; + #size-cells = <0>; + pinctrl-0 = <&joystick_pins>; + pinctrl-names = "default"; + button@0 { + label = "JoySel"; + linux,code = <KEY_ENTER>; + interrupt-parent = <&stmfxgpio>; + interrupts = <0 IRQ_TYPE_EDGE_RISING>; + }; + button@1 { + label = "JoyDown"; + linux,code = <KEY_DOWN>; + interrupt-parent = <&stmfxgpio>; + interrupts = <1 IRQ_TYPE_EDGE_RISING>; + }; + button@2 { + label = "JoyLeft"; + linux,code = <KEY_LEFT>; + interrupt-parent = <&stmfxgpio>; + interrupts = <2 IRQ_TYPE_EDGE_RISING>; + }; + button@3 { + label = "JoyRight"; + linux,code = <KEY_RIGHT>; + interrupt-parent = <&stmfxgpio>; + interrupts = <3 IRQ_TYPE_EDGE_RISING>; + }; + button@4 { + label = "JoyUp"; + linux,code = <KEY_UP>; + interrupt-parent = <&stmfxgpio>; + interrupts = <4 IRQ_TYPE_EDGE_RISING>; + }; + }; + + leds { + compatible = "gpio-leds"; + orange { + gpios = <&stmfxgpio 17 1>; + }; + + blue { + gpios = <&stmfxgpio 19 1>; + }; + }
This patch adds documentation of device tree bindings for the STMicroelectronics Multi-Function eXpander (STMFX) GPIO expander. Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com> --- .../devicetree/bindings/pinctrl/pinctrl-stmfx.txt | 118 +++++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/pinctrl-stmfx.txt