| Message ID | 1518100057-23234-2-git-send-email-amelie.delaunay@st.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Feb 08, 2018 at 03:27:32PM +0100, Amelie Delaunay wrote: > This patch adds documentation of device tree bindings for the > STMicroelectronics Multi-Function eXpander (MFX). > > Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com> > --- > Documentation/devicetree/bindings/mfd/st-mfx.txt | 51 ++++++++++++++++++++++++ > 1 file changed, 51 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mfd/st-mfx.txt > > diff --git a/Documentation/devicetree/bindings/mfd/st-mfx.txt b/Documentation/devicetree/bindings/mfd/st-mfx.txt > new file mode 100644 > index 0000000..423d800 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mfd/st-mfx.txt > @@ -0,0 +1,51 @@ > +STMicroelectronics Multi-Function eXpander > + > +ST Multi-Function eXpander (MFX) is a slave controller using I2C for > +communication with the main MCU. Its main features are gpio expansion, main > +MCU IDD measurement (IDD is the amount of current that flows through VDD) > +and resistive touchscreen controller. You don't have to implement all the drivers now, but please completely describe the device. As is, there is no reason to have a child GPIO node. > + > +Required properties: > +- compatible: must be "st,mfx" Kind of generic. Only 1 single version ever? > +- reg: I2C address of the device > +- interrupts: interrupt triggered by MFX_IRQ_OUT signal > +- interrupt-parent: interrupt controller MFX is connected to > +- interrupt-controller: marks the device as an interrupt controller > +- #interrupt-cells: should be <1>, index of the interrupt within the > + controller, in accordance with the "one cell" variant of > + <devicetree/bindings/interrupt-controller/interrupt.txt> > + > +Optional nodes: > + > +* GPIO eXpander > +MFX provides 16 programmable GPIOs, and it is also possible to recover 8 > +alternate GPIOs if the main functions are not used (touchscreen controller and > +IDD measurement not enabled). > + > +Required properties: > +- compatible : must be "st,mfx-gpio" > +- interrupt-parent : must be <&mfx> Not necessary. A parent node with 'interrupt-controller' property is the interrupt's parent. > +- interrupts = must be <0> > +- gpio-controller: marks the device node as a GPIO controller > +- #gpio-cells: should be <2>, the first cell is the GPIO offset on this GPIO > + controller, the second cell is the gpio flags in accordance with > + <dt-bindings/gpio/st-mfx-gpio.h>. Custom flags? Use standard flags. DT binding headers should be part of this patch. > + > +Example: > + > + mfx: mfx@42 { > + compatible = "st,mfx"; > + reg = <0x42>; > + interrupts = <8 IRQ_TYPE_EDGE_RISING>; > + interrupt-parent = <&gpioi>; > + interrupt-controller; > + #interrupt-cells = <1>; > + > + mfxgpio: mfx_gpio { gpio { > + compatible = "st,mfx-gpio"; > + interrupt-parent = <&mfx>; > + interrupts = <0>; > + gpio-controller; > + #gpio-cells = <2>; > + }; > + }; > -- > 2.7.4 >
On 02/19/2018 12:19 AM, Rob Herring wrote: > On Thu, Feb 08, 2018 at 03:27:32PM +0100, Amelie Delaunay wrote: >> This patch adds documentation of device tree bindings for the >> STMicroelectronics Multi-Function eXpander (MFX). >> >> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com> >> --- >> Documentation/devicetree/bindings/mfd/st-mfx.txt | 51 ++++++++++++++++++++++++ >> 1 file changed, 51 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/mfd/st-mfx.txt >> >> diff --git a/Documentation/devicetree/bindings/mfd/st-mfx.txt b/Documentation/devicetree/bindings/mfd/st-mfx.txt >> new file mode 100644 >> index 0000000..423d800 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/mfd/st-mfx.txt >> @@ -0,0 +1,51 @@ >> +STMicroelectronics Multi-Function eXpander >> + >> +ST Multi-Function eXpander (MFX) is a slave controller using I2C for >> +communication with the main MCU. Its main features are gpio expansion, main >> +MCU IDD measurement (IDD is the amount of current that flows through VDD) >> +and resistive touchscreen controller. > > You don't have to implement all the drivers now, but please completely > describe the device. As is, there is no reason to have a child GPIO > node. > The MFD driver will be abandoned as only GPIO part is used. I'll send a V2 soon. >> + >> +Required properties: >> +- compatible: must be "st,mfx" > > Kind of generic. Only 1 single version ever? > "st-mfx" compatible will disappear in V2 (we keep only GPIO driver). MFX version can be read in MFX FW_VERSION register. So do I keep "st,mfx-gpio" compatible or you want to see mfx version ? >> +- reg: I2C address of the device >> +- interrupts: interrupt triggered by MFX_IRQ_OUT signal >> +- interrupt-parent: interrupt controller MFX is connected to >> +- interrupt-controller: marks the device as an interrupt controller >> +- #interrupt-cells: should be <1>, index of the interrupt within the >> + controller, in accordance with the "one cell" variant of >> + <devicetree/bindings/interrupt-controller/interrupt.txt> >> + >> +Optional nodes: >> + >> +* GPIO eXpander >> +MFX provides 16 programmable GPIOs, and it is also possible to recover 8 >> +alternate GPIOs if the main functions are not used (touchscreen controller and >> +IDD measurement not enabled). >> + >> +Required properties: >> +- compatible : must be "st,mfx-gpio" >> +- interrupt-parent : must be <&mfx> > > Not necessary. A parent node with 'interrupt-controller' property is the > interrupt's parent. > I will keep it in mind. >> +- interrupts = must be <0> >> +- gpio-controller: marks the device node as a GPIO controller >> +- #gpio-cells: should be <2>, the first cell is the GPIO offset on this GPIO >> + controller, the second cell is the gpio flags in accordance with >> + <dt-bindings/gpio/st-mfx-gpio.h>. > > Custom flags? Use standard flags. > > DT binding headers should be part of this patch. > Custom flags because MFX GPIOs have several types: - Output open drain with internal pull-up resistor - Output open drain without internal pull-up resistor - Output push pull without internal pull resistor - Input with internal pull-up resistor - Input with internal pull-down resistor - Input floating - Input analog. Standard flags don't have pull up or down information. That's why I am using custom flags, they overloads standard flags. I will add DT bindings header in this patch V2. >> + >> +Example: >> + >> + mfx: mfx@42 { >> + compatible = "st,mfx"; >> + reg = <0x42>; >> + interrupts = <8 IRQ_TYPE_EDGE_RISING>; >> + interrupt-parent = <&gpioi>; >> + interrupt-controller; >> + #interrupt-cells = <1>; >> + >> + mfxgpio: mfx_gpio { > > gpio { > OK. Will change it in V2. Thanks, Amelie >> + compatible = "st,mfx-gpio"; >> + interrupt-parent = <&mfx>; >> + interrupts = <0>; >> + gpio-controller; >> + #gpio-cells = <2>; >> + }; >> + }; >> -- >> 2.7.4 >>
On Mon, Feb 19, 2018 at 9:59 AM, Amelie DELAUNAY <amelie.delaunay@st.com> wrote: > > > On 02/19/2018 12:19 AM, Rob Herring wrote: >> On Thu, Feb 08, 2018 at 03:27:32PM +0100, Amelie Delaunay wrote: >>> This patch adds documentation of device tree bindings for the >>> STMicroelectronics Multi-Function eXpander (MFX). >>> >>> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com> >>> --- >>> Documentation/devicetree/bindings/mfd/st-mfx.txt | 51 ++++++++++++++++++++++++ >>> 1 file changed, 51 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/mfd/st-mfx.txt >>> >>> diff --git a/Documentation/devicetree/bindings/mfd/st-mfx.txt b/Documentation/devicetree/bindings/mfd/st-mfx.txt >>> new file mode 100644 >>> index 0000000..423d800 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/mfd/st-mfx.txt >>> @@ -0,0 +1,51 @@ >>> +STMicroelectronics Multi-Function eXpander >>> + >>> +ST Multi-Function eXpander (MFX) is a slave controller using I2C for >>> +communication with the main MCU. Its main features are gpio expansion, main >>> +MCU IDD measurement (IDD is the amount of current that flows through VDD) >>> +and resistive touchscreen controller. >> >> You don't have to implement all the drivers now, but please completely >> describe the device. As is, there is no reason to have a child GPIO >> node. >> > The MFD driver will be abandoned as only GPIO part is used. I'll send a > V2 soon. >>> + >>> +Required properties: >>> +- compatible: must be "st,mfx" >> >> Kind of generic. Only 1 single version ever? >> > "st-mfx" compatible will disappear in V2 (we keep only GPIO driver). MFX > version can be read in MFX FW_VERSION register. So do I keep > "st,mfx-gpio" compatible or you want to see mfx version ? That sounds a bit worrying. The other functions will *never* get accessed? The DT should reflect the h/w including any future needs, not just what you have a driver for today. >>> +- reg: I2C address of the device >>> +- interrupts: interrupt triggered by MFX_IRQ_OUT signal >>> +- interrupt-parent: interrupt controller MFX is connected to >>> +- interrupt-controller: marks the device as an interrupt controller >>> +- #interrupt-cells: should be <1>, index of the interrupt within the >>> + controller, in accordance with the "one cell" variant of >>> + <devicetree/bindings/interrupt-controller/interrupt.txt> >>> + >>> +Optional nodes: >>> + >>> +* GPIO eXpander >>> +MFX provides 16 programmable GPIOs, and it is also possible to recover 8 >>> +alternate GPIOs if the main functions are not used (touchscreen controller and >>> +IDD measurement not enabled). >>> + >>> +Required properties: >>> +- compatible : must be "st,mfx-gpio" >>> +- interrupt-parent : must be <&mfx> >> >> Not necessary. A parent node with 'interrupt-controller' property is the >> interrupt's parent. >> > I will keep it in mind. >>> +- interrupts = must be <0> >>> +- gpio-controller: marks the device node as a GPIO controller >>> +- #gpio-cells: should be <2>, the first cell is the GPIO offset on this GPIO >>> + controller, the second cell is the gpio flags in accordance with >>> + <dt-bindings/gpio/st-mfx-gpio.h>. >> >> Custom flags? Use standard flags. >> >> DT binding headers should be part of this patch. >> > Custom flags because MFX GPIOs have several types: > - Output open drain with internal pull-up resistor > - Output open drain without internal pull-up resistor > - Output push pull without internal pull resistor > - Input with internal pull-up resistor > - Input with internal pull-down resistor > - Input floating > - Input analog. > Standard flags don't have pull up or down information. That's why I am > using custom flags, they overloads standard flags. I'll leave this one to Linus to comment on. > I will add DT bindings header in this patch V2.
On Thu, Feb 8, 2018 at 3:27 PM, Amelie Delaunay <amelie.delaunay@st.com> wrote: > +Required properties: > +- compatible: must be "st,mfx" I bet this should be more specific. Tomorrow there will be a new version of this expander and then we will wish that we used a more specific compatible for the first one. Does this chip have any kind of product number on it? Else I would be tempted to use the compatible "st,mfx-0000" or something, indicating it is the first of its kind. > +- reg: I2C address of the device > +- interrupts: interrupt triggered by MFX_IRQ_OUT signal > +- interrupt-parent: interrupt controller MFX is connected to > +- interrupt-controller: marks the device as an interrupt controller > +- #interrupt-cells: should be <1>, index of the interrupt within the > + controller, in accordance with the "one cell" variant of > + <devicetree/bindings/interrupt-controller/interrupt.txt> Seems fine. > +Optional nodes: > + > +* GPIO eXpander > +MFX provides 16 programmable GPIOs, and it is also possible to recover 8 > +alternate GPIOs if the main functions are not used (touchscreen controller and > +IDD measurement not enabled). Apparenly Rob thinks this should go elsewhere. > +- gpio-controller: marks the device node as a GPIO controller > +- #gpio-cells: should be <2>, the first cell is the GPIO offset on this GPIO > + controller, the second cell is the gpio flags in accordance with > + <dt-bindings/gpio/st-mfx-gpio.h>. Let's discuss these extra GPIO flag bindings separately. Yours, Linus Walleij
On Mon, Feb 19, 2018 at 4:59 PM, Amelie DELAUNAY <amelie.delaunay@st.com> wrote: > On 02/19/2018 12:19 AM, Rob Herring wrote: >> On Thu, Feb 08, 2018 at 03:27:32PM +0100, Amelie Delaunay wrote: >>> +- interrupts = must be <0> >>> +- gpio-controller: marks the device node as a GPIO controller >>> +- #gpio-cells: should be <2>, the first cell is the GPIO offset on this GPIO >>> + controller, the second cell is the gpio flags in accordance with >>> + <dt-bindings/gpio/st-mfx-gpio.h>. >> >> Custom flags? Use standard flags. >> >> DT binding headers should be part of this patch. >> > Custom flags because MFX GPIOs have several types: > - Output open drain with internal pull-up resistor > - Output open drain without internal pull-up resistor > - Output push pull without internal pull resistor > - Input with internal pull-up resistor > - Input with internal pull-down resistor > - Input floating > - Input analog. > Standard flags don't have pull up or down information. That's why I am > using custom flags, they overloads standard flags. This is because pull up/down and tristate/high z (floating) also known as PIN_CONFIG_BIAS_HIGH_IMPEDANCE, is controlled by the pin control subsystem, not GPIO. If your hardware does even more pin control (like multiplexing) then I suggest that you create a pin control driver back-end for this and put the resulting driver in drivers/pinctrl. Some recent additions of expanders in drivers/pinctrl makes for great inspiration for this work. See for example: drivers/pinctrl/pinctrl-sx150x.c drivers/pinctrl/pinctrl-mcp23s08.c These are both combined pin control and GPIO drivers that we moved from drivers/gpio because we concluded that they do more than just GPIO. The GPIO lines are matches to pins using the GPIO ranges from the call to gpiochip_add_pin_range() and using gpiochip_generic_config() as the gpiochip .set_config() callback. It has been discussed to expose subsets of pin config to GPIO. Indeed, we already handle open drain/source and debounce by simply calling into the pin control back-end or handling it directly in the GPIO driver using the standard pin config properties. I am reluctant about this, I think the split of responsibilities is pretty nice. I'd recommend looking at the sx150x pin control driver and check if you can maybe take this direction with the patch? Yours, Linus Walleij
On 02/22/2018 02:22 PM, Linus Walleij wrote: > On Mon, Feb 19, 2018 at 4:59 PM, Amelie DELAUNAY <amelie.delaunay@st.com> wrote: >> On 02/19/2018 12:19 AM, Rob Herring wrote: >>> On Thu, Feb 08, 2018 at 03:27:32PM +0100, Amelie Delaunay wrote: > >>>> +- interrupts = must be <0> >>>> +- gpio-controller: marks the device node as a GPIO controller >>>> +- #gpio-cells: should be <2>, the first cell is the GPIO offset on this GPIO >>>> + controller, the second cell is the gpio flags in accordance with >>>> + <dt-bindings/gpio/st-mfx-gpio.h>. >>> >>> Custom flags? Use standard flags. >>> >>> DT binding headers should be part of this patch. >>> >> Custom flags because MFX GPIOs have several types: >> - Output open drain with internal pull-up resistor >> - Output open drain without internal pull-up resistor >> - Output push pull without internal pull resistor >> - Input with internal pull-up resistor >> - Input with internal pull-down resistor >> - Input floating >> - Input analog. >> Standard flags don't have pull up or down information. That's why I am >> using custom flags, they overloads standard flags. > > This is because pull up/down and tristate/high z (floating) > also known as PIN_CONFIG_BIAS_HIGH_IMPEDANCE, > is controlled by the pin control subsystem, not GPIO. > > If your hardware does even more pin control (like multiplexing) > then I suggest that you create a pin control driver back-end for > this and put the resulting driver in drivers/pinctrl. > > Some recent additions of expanders in drivers/pinctrl makes > for great inspiration for this work. See for example: > drivers/pinctrl/pinctrl-sx150x.c > drivers/pinctrl/pinctrl-mcp23s08.c > > These are both combined pin control and GPIO drivers that > we moved from drivers/gpio because we concluded that they > do more than just GPIO. The GPIO lines are matches to > pins using the GPIO ranges from the call to > gpiochip_add_pin_range() and using gpiochip_generic_config() > as the gpiochip .set_config() callback. > > It has been discussed to expose subsets of pin config to > GPIO. Indeed, we already handle open drain/source and > debounce by simply calling into the pin control back-end > or handling it directly in the GPIO driver using the standard > pin config properties. > > I am reluctant about this, I think the split of responsibilities > is pretty nice. > > I'd recommend looking at the sx150x pin control driver and > check if you can maybe take this direction with the patch? > Thanks for highlighting these examples. I will have a look on these! Regards, Amelie > Yours, > Linus Walleij >
On 02/22/2018 02:11 PM, Linus Walleij wrote: > On Thu, Feb 8, 2018 at 3:27 PM, Amelie Delaunay <amelie.delaunay@st.com> wrote: > >> +Required properties: >> +- compatible: must be "st,mfx" > > I bet this should be more specific. Tomorrow there will be a new > version of this expander and then we will wish that we used > a more specific compatible for the first one. > > Does this chip have any kind of product number on it? > > Else I would be tempted to use the compatible "st,mfx-0000" > or something, indicating it is the first of its kind. > This chip has a FW version. So, I agree, I should use a compatible that reflects this FW version. >> +- reg: I2C address of the device >> +- interrupts: interrupt triggered by MFX_IRQ_OUT signal >> +- interrupt-parent: interrupt controller MFX is connected to >> +- interrupt-controller: marks the device as an interrupt controller >> +- #interrupt-cells: should be <1>, index of the interrupt within the >> + controller, in accordance with the "one cell" variant of >> + <devicetree/bindings/interrupt-controller/interrupt.txt> > > Seems fine. > >> +Optional nodes: >> + >> +* GPIO eXpander >> +MFX provides 16 programmable GPIOs, and it is also possible to recover 8 >> +alternate GPIOs if the main functions are not used (touchscreen controller and >> +IDD measurement not enabled). > > Apparenly Rob thinks this should go elsewhere. > It would go with gpio/pinctrl driver, so in dt-bindings/(gpio|pinctrl). Regards, Amelie >> +- gpio-controller: marks the device node as a GPIO controller >> +- #gpio-cells: should be <2>, the first cell is the GPIO offset on this GPIO >> + controller, the second cell is the gpio flags in accordance with >> + <dt-bindings/gpio/st-mfx-gpio.h>. > > Let's discuss these extra GPIO flag bindings separately. > > Yours, > Linus Walleij >
diff --git a/Documentation/devicetree/bindings/mfd/st-mfx.txt b/Documentation/devicetree/bindings/mfd/st-mfx.txt new file mode 100644 index 0000000..423d800 --- /dev/null +++ b/Documentation/devicetree/bindings/mfd/st-mfx.txt @@ -0,0 +1,51 @@ +STMicroelectronics Multi-Function eXpander + +ST Multi-Function eXpander (MFX) is a slave controller using I2C for +communication with the main MCU. Its main features are gpio expansion, main +MCU IDD measurement (IDD is the amount of current that flows through VDD) +and resistive touchscreen controller. + +Required properties: +- compatible: must be "st,mfx" +- reg: I2C address of the device +- interrupts: interrupt triggered by MFX_IRQ_OUT signal +- interrupt-parent: interrupt controller MFX is connected to +- interrupt-controller: marks the device as an interrupt controller +- #interrupt-cells: should be <1>, index of the interrupt within the + controller, in accordance with the "one cell" variant of + <devicetree/bindings/interrupt-controller/interrupt.txt> + +Optional nodes: + +* GPIO eXpander +MFX provides 16 programmable GPIOs, and it is also possible to recover 8 +alternate GPIOs if the main functions are not used (touchscreen controller and +IDD measurement not enabled). + +Required properties: +- compatible : must be "st,mfx-gpio" +- interrupt-parent : must be <&mfx> +- interrupts = must be <0> +- gpio-controller: marks the device node as a GPIO controller +- #gpio-cells: should be <2>, the first cell is the GPIO offset on this GPIO + controller, the second cell is the gpio flags in accordance with + <dt-bindings/gpio/st-mfx-gpio.h>. + +Example: + + mfx: mfx@42 { + compatible = "st,mfx"; + reg = <0x42>; + interrupts = <8 IRQ_TYPE_EDGE_RISING>; + interrupt-parent = <&gpioi>; + interrupt-controller; + #interrupt-cells = <1>; + + mfxgpio: mfx_gpio { + compatible = "st,mfx-gpio"; + interrupt-parent = <&mfx>; + interrupts = <0>; + gpio-controller; + #gpio-cells = <2>; + }; + };
This patch adds documentation of device tree bindings for the STMicroelectronics Multi-Function eXpander (MFX). Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com> --- Documentation/devicetree/bindings/mfd/st-mfx.txt | 51 ++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 Documentation/devicetree/bindings/mfd/st-mfx.txt