| Message ID | 1545984581-25843-6-git-send-email-erin.lo@mediatek.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | Add basic and clock support for Mediatek MT8183 SoC | expand |
On Fri, Dec 28, 2018 at 04:09:40PM +0800, Erin Lo wrote: > From: Zhiyong Tao <zhiyong.tao@mediatek.com> > > The commit adds mt8183 compatible node in binding document. > > Signed-off-by: Zhiyong Tao <zhiyong.tao@mediatek.com> > Signed-off-by: Erin Lo <erin.lo@mediatek.com> > --- > .../devicetree/bindings/pinctrl/pinctrl-mt8183.txt | 110 +++++++++++++++++++++ > 1 file changed, 110 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt > > diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt > new file mode 100644 > index 0000000..7b5285e > --- /dev/null > +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt > @@ -0,0 +1,110 @@ > +* Mediatek MT8183 Pin Controller > + > +The Mediatek's Pin controller is used to control SoC pins. > + > +Required properties: > +- compatible: value should be one of the following. > + "mediatek,mt8183-pinctrl", compatible with mt8183 pinctrl. > +- gpio-controller : Marks the device node as a gpio controller. > +- #gpio-cells: number of cells in GPIO specifier. Since the generic GPIO > + binding is used, the amount of cells must be specified as 2. See the below > + mentioned gpio binding representation for description of particular cells. > +- gpio-ranges : gpio valid number range. > + > + Eg: <&pio 6 0> > + <[phandle of the gpio controller node] > + [line number within the gpio controller] > + [flags]> > + > + Values for gpio specifier: > + - Line number: is a value between 0 to 202. > + - Flags: bit field of flags, as defined in <dt-bindings/gpio/gpio.h>. > + Only the following flags are supported: > + 0 - GPIO_ACTIVE_HIGH > + 1 - GPIO_ACTIVE_LOW > + > +Optional properties: > +- reg: physicall address base for gpio base registers. s/physicall/physical/ reg should never be optional. Need to say how many reg entries. > +- reg-names: gpio base registers name. Need to say what are the names. However, I don't find the names in the example all that useful, so I'd just drop it. > +- interrupt-controller: Marks the device node as an interrupt controller > +- #interrupt-cells: Should be two. > +- interrupts : The interrupt outputs from the controller. > + > +Please refer to pinctrl-bindings.txt in this directory for details of the > +common pinctrl bindings used by client devices. > + > +Subnode format > +A pinctrl node should contain at least one subnodes representing the > +pinctrl groups available on the machine. Each subnode will list the > +pins it needs, and how they should be configured, with regard to muxer > +configuration, pullups, drive strength, input enable/disable and input schmitt. > + > + node { > + pinmux = <PIN_NUMBER_PINMUX>; > + GENERIC_PINCONFIG; > + }; > + > +Required properties: > +- pinmux: integer array, represents gpio pin number and mux setting. > + Supported pin number and mux varies for different SoCs, and are defined > + as macros in boot/dts/<soc>-pinfunc.h directly. > + > +Optional properties: > +- GENERIC_PINCONFIG: is the generic pinconfig options to use, bias-disable, > + bias-pull-down, bias-pull-up, input-enable, input-disable, output-low, output-high, > + input-schmitt-enable, input-schmitt-disable and drive-strength are valid. > + > + Some special pins have extra pull up strength, there are R0 and R1 pull-up > + resistors available, but for user, it's only need to set R1R0 as 00, 01, 10 or 11. > + So when config mediatek,pull-up-adv or mediatek,pull-down-adv, > + it support arguments for those special pins. > + > + When config drive-strength, it can support some arguments, such as > + MTK_DRIVE_4mA, MTK_DRIVE_6mA, etc. See dt-bindings/pinctrl/mt65xx.h. > + > +Examples: > + > +#include "mt8183-pinfunc.h" > + > +... > +{ > + pio: pinctrl@10005000 { > + compatible = "mediatek,mt8183-pinctrl"; > + reg = <0 0x10005000 0 0x1000>, > + <0 0x11F20000 0 0x1000>, > + <0 0x11E80000 0 0x1000>, > + <0 0x11E70000 0 0x1000>, > + <0 0x11E90000 0 0x1000>, > + <0 0x11D30000 0 0x1000>, > + <0 0x11D20000 0 0x1000>, > + <0 0x11C50000 0 0x1000>, > + <0 0x11F30000 0 0x1000>; > + reg-names = "iocfg0", "iocfg1", "iocfg2", > + "iocfg3", "iocfg4", "iocfg5", > + "iocfg6", "iocfg7", "iocfg8"; > + gpio-controller; > + #gpio-cells = <2>; > + gpio-ranges = <&pio 0 0 192>; > + interrupt-controller; > + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>; > + interrupt-parent = <&gic>; > + #interrupt-cells = <2>; > + > + i2c0_pins_a: i2c0@0 { unit-address without reg property is not valid. > + pins1 { > + pinmux = <PINMUX_GPIO48__FUNC_SCL5>, > + <PINMUX_GPIO49__FUNC_SDA5>; > + mediatek,pull-up-adv = <11>; > + }; > + }; > + > + i2c1_pins_a: i2c1@0 { > + pins { > + pinmux = <PINMUX_GPIO50__FUNC_SCL3>, > + <PINMUX_GPIO51__FUNC_SDA3>; > + mediatek,pull-down-adv = <10>; > + }; > + }; > + ... > + }; > +}; > -- > 1.9.1 >
On Sat, 2018-12-29 at 06:04 +0800, Rob Herring wrote: > On Fri, Dec 28, 2018 at 04:09:40PM +0800, Erin Lo wrote: > > From: Zhiyong Tao <zhiyong.tao@mediatek.com> > > > > The commit adds mt8183 compatible node in binding document. > > > > Signed-off-by: Zhiyong Tao <zhiyong.tao@mediatek.com> > > Signed-off-by: Erin Lo <erin.lo@mediatek.com> > > --- > > .../devicetree/bindings/pinctrl/pinctrl-mt8183.txt | 110 +++++++++++++++++++++ > > 1 file changed, 110 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt > > > > diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt > > new file mode 100644 > > index 0000000..7b5285e > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt > > @@ -0,0 +1,110 @@ > > +* Mediatek MT8183 Pin Controller > > + > > +The Mediatek's Pin controller is used to control SoC pins. > > + > > +Required properties: > > +- compatible: value should be one of the following. > > + "mediatek,mt8183-pinctrl", compatible with mt8183 pinctrl. > > +- gpio-controller : Marks the device node as a gpio controller. > > +- #gpio-cells: number of cells in GPIO specifier. Since the generic GPIO > > + binding is used, the amount of cells must be specified as 2. See the below > > + mentioned gpio binding representation for description of particular cells. > > +- gpio-ranges : gpio valid number range. > > + > > + Eg: <&pio 6 0> > > + <[phandle of the gpio controller node] > > + [line number within the gpio controller] > > + [flags]> > > + > > + Values for gpio specifier: > > + - Line number: is a value between 0 to 202. > > + - Flags: bit field of flags, as defined in <dt-bindings/gpio/gpio.h>. > > + Only the following flags are supported: > > + 0 - GPIO_ACTIVE_HIGH > > + 1 - GPIO_ACTIVE_LOW > > + > > +Optional properties: > > +- reg: physicall address base for gpio base registers. > > s/physicall/physical/ > > reg should never be optional. > > Need to say how many reg entries. ==> Thanks for your suggestion. We will change 'reg' to Required properties and add the reg entries in next version. > > > +- reg-names: gpio base registers name. > > Need to say what are the names. However, I don't find the names in the > example all that useful, so I'd just drop it. ==> we will add the reg-names in next version. They are used in the sample code, such as: > + reg-names = "iocfg0", "iocfg1", "iocfg2", > > + "iocfg3", "iocfg4", "iocfg5", > > + "iocfg6", "iocfg7", "iocfg8"; > > > +- interrupt-controller: Marks the device node as an interrupt controller > > +- #interrupt-cells: Should be two. > > +- interrupts : The interrupt outputs from the controller. > > + > > +Please refer to pinctrl-bindings.txt in this directory for details of the > > +common pinctrl bindings used by client devices. > > + > > +Subnode format > > +A pinctrl node should contain at least one subnodes representing the > > +pinctrl groups available on the machine. Each subnode will list the > > +pins it needs, and how they should be configured, with regard to muxer > > +configuration, pullups, drive strength, input enable/disable and input schmitt. > > + > > + node { > > + pinmux = <PIN_NUMBER_PINMUX>; > > + GENERIC_PINCONFIG; > > + }; > > + > > +Required properties: > > +- pinmux: integer array, represents gpio pin number and mux setting. > > + Supported pin number and mux varies for different SoCs, and are defined > > + as macros in boot/dts/<soc>-pinfunc.h directly. > > + > > +Optional properties: > > +- GENERIC_PINCONFIG: is the generic pinconfig options to use, bias-disable, > > + bias-pull-down, bias-pull-up, input-enable, input-disable, output-low, output-high, > > + input-schmitt-enable, input-schmitt-disable and drive-strength are valid. > > + > > + Some special pins have extra pull up strength, there are R0 and R1 pull-up > > + resistors available, but for user, it's only need to set R1R0 as 00, 01, 10 or 11. > > + So when config mediatek,pull-up-adv or mediatek,pull-down-adv, > > + it support arguments for those special pins. > > + > > + When config drive-strength, it can support some arguments, such as > > + MTK_DRIVE_4mA, MTK_DRIVE_6mA, etc. See dt-bindings/pinctrl/mt65xx.h. > > + > > +Examples: > > + > > +#include "mt8183-pinfunc.h" > > + > > +... > > +{ > > + pio: pinctrl@10005000 { > > + compatible = "mediatek,mt8183-pinctrl"; > > + reg = <0 0x10005000 0 0x1000>, > > + <0 0x11F20000 0 0x1000>, > > + <0 0x11E80000 0 0x1000>, > > + <0 0x11E70000 0 0x1000>, > > + <0 0x11E90000 0 0x1000>, > > + <0 0x11D30000 0 0x1000>, > > + <0 0x11D20000 0 0x1000>, > > + <0 0x11C50000 0 0x1000>, > > + <0 0x11F30000 0 0x1000>; > > + reg-names = "iocfg0", "iocfg1", "iocfg2", > > + "iocfg3", "iocfg4", "iocfg5", > > + "iocfg6", "iocfg7", "iocfg8"; > > + gpio-controller; > > + #gpio-cells = <2>; > > + gpio-ranges = <&pio 0 0 192>; > > + interrupt-controller; > > + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>; > > + interrupt-parent = <&gic>; > > + #interrupt-cells = <2>; > > + > > + i2c0_pins_a: i2c0@0 { > > unit-address without reg property is not valid. ==> we will change "i2c0_pins_a: i2c0@0" to "i2c0_pins_a: i2c0" in the next version. > > > + pins1 { > > + pinmux = <PINMUX_GPIO48__FUNC_SCL5>, > > + <PINMUX_GPIO49__FUNC_SDA5>; > > + mediatek,pull-up-adv = <11>; > > + }; > > + }; > > + > > + i2c1_pins_a: i2c1@0 { > > + pins { > > + pinmux = <PINMUX_GPIO50__FUNC_SCL3>, > > + <PINMUX_GPIO51__FUNC_SDA3>; > > + mediatek,pull-down-adv = <10>; > > + }; > > + }; > > + ... > > + }; > > +}; > > -- > > 1.9.1 > >
On Thu, Jan 3, 2019 at 11:09 PM Zhiyong Tao <zhiyong.tao@mediatek.com> wrote: > > On Sat, 2018-12-29 at 06:04 +0800, Rob Herring wrote: > > On Fri, Dec 28, 2018 at 04:09:40PM +0800, Erin Lo wrote: > > > From: Zhiyong Tao <zhiyong.tao@mediatek.com> > > > > > > The commit adds mt8183 compatible node in binding document. > > > > > > Signed-off-by: Zhiyong Tao <zhiyong.tao@mediatek.com> > > > Signed-off-by: Erin Lo <erin.lo@mediatek.com> > > > --- > > > .../devicetree/bindings/pinctrl/pinctrl-mt8183.txt | 110 +++++++++++++++++++++ > > > 1 file changed, 110 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt > > > > > > diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt > > > new file mode 100644 > > > index 0000000..7b5285e > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt > > > @@ -0,0 +1,110 @@ > > > +* Mediatek MT8183 Pin Controller > > > + > > > +The Mediatek's Pin controller is used to control SoC pins. > > > + > > > +Required properties: > > > +- compatible: value should be one of the following. > > > + "mediatek,mt8183-pinctrl", compatible with mt8183 pinctrl. > > > +- gpio-controller : Marks the device node as a gpio controller. > > > +- #gpio-cells: number of cells in GPIO specifier. Since the generic GPIO > > > + binding is used, the amount of cells must be specified as 2. See the below > > > + mentioned gpio binding representation for description of particular cells. > > > +- gpio-ranges : gpio valid number range. > > > + > > > + Eg: <&pio 6 0> > > > + <[phandle of the gpio controller node] > > > + [line number within the gpio controller] > > > + [flags]> > > > + > > > + Values for gpio specifier: > > > + - Line number: is a value between 0 to 202. > > > + - Flags: bit field of flags, as defined in <dt-bindings/gpio/gpio.h>. > > > + Only the following flags are supported: > > > + 0 - GPIO_ACTIVE_HIGH > > > + 1 - GPIO_ACTIVE_LOW > > > + > > > +Optional properties: > > > +- reg: physicall address base for gpio base registers. > > > > s/physicall/physical/ > > > > reg should never be optional. > > > > Need to say how many reg entries. > > ==> Thanks for your suggestion. We will change 'reg' to Required > properties and add the reg entries in next version. > > > > > +- reg-names: gpio base registers name. > > > > Need to say what are the names. However, I don't find the names in the > > example all that useful, so I'd just drop it. > > ==> we will add the reg-names in next version. > They are used in the sample code, such as: > > + reg-names = "iocfg0", "iocfg1", "iocfg2", > > > + "iocfg3", "iocfg4", "iocfg5", > > > + "iocfg6", "iocfg7", "iocfg8"; > > > > > > +- interrupt-controller: Marks the device node as an interrupt controller > > > +- #interrupt-cells: Should be two. > > > +- interrupts : The interrupt outputs from the controller. > > > + > > > +Please refer to pinctrl-bindings.txt in this directory for details of the > > > +common pinctrl bindings used by client devices. > > > + > > > +Subnode format > > > +A pinctrl node should contain at least one subnodes representing the > > > +pinctrl groups available on the machine. Each subnode will list the > > > +pins it needs, and how they should be configured, with regard to muxer > > > +configuration, pullups, drive strength, input enable/disable and input schmitt. > > > + > > > + node { > > > + pinmux = <PIN_NUMBER_PINMUX>; > > > + GENERIC_PINCONFIG; > > > + }; > > > + > > > +Required properties: > > > +- pinmux: integer array, represents gpio pin number and mux setting. > > > + Supported pin number and mux varies for different SoCs, and are defined > > > + as macros in boot/dts/<soc>-pinfunc.h directly. > > > + > > > +Optional properties: > > > +- GENERIC_PINCONFIG: is the generic pinconfig options to use, bias-disable, > > > + bias-pull-down, bias-pull-up, input-enable, input-disable, output-low, output-high, > > > + input-schmitt-enable, input-schmitt-disable and drive-strength are valid. > > > + > > > + Some special pins have extra pull up strength, there are R0 and R1 pull-up > > > + resistors available, but for user, it's only need to set R1R0 as 00, 01, 10 or 11. > > > + So when config mediatek,pull-up-adv or mediatek,pull-down-adv, > > > + it support arguments for those special pins. > > > + > > > + When config drive-strength, it can support some arguments, such as > > > + MTK_DRIVE_4mA, MTK_DRIVE_6mA, etc. See dt-bindings/pinctrl/mt65xx.h. > > > + One point we can fix more is the drive-strength already is supported as the generic one, not need to depend on a dedicated header file. > > > +Examples: > > > + > > > +#include "mt8183-pinfunc.h" > > > + > > > +... > > > +{ > > > + pio: pinctrl@10005000 { > > > + compatible = "mediatek,mt8183-pinctrl"; > > > + reg = <0 0x10005000 0 0x1000>, > > > + <0 0x11F20000 0 0x1000>, > > > + <0 0x11E80000 0 0x1000>, > > > + <0 0x11E70000 0 0x1000>, > > > + <0 0x11E90000 0 0x1000>, > > > + <0 0x11D30000 0 0x1000>, > > > + <0 0x11D20000 0 0x1000>, > > > + <0 0x11C50000 0 0x1000>, > > > + <0 0x11F30000 0 0x1000>; > > > + reg-names = "iocfg0", "iocfg1", "iocfg2", > > > + "iocfg3", "iocfg4", "iocfg5", > > > + "iocfg6", "iocfg7", "iocfg8"; > > > + gpio-controller; > > > + #gpio-cells = <2>; > > > + gpio-ranges = <&pio 0 0 192>; > > > + interrupt-controller; > > > + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>; > > > + interrupt-parent = <&gic>; > > > + #interrupt-cells = <2>; > > > + > > > + i2c0_pins_a: i2c0@0 { > > > > unit-address without reg property is not valid. > > ==> we will change "i2c0_pins_a: i2c0@0" to "i2c0_pins_a: i2c0" in the > next version. > > > > > + pins1 { > > > + pinmux = <PINMUX_GPIO48__FUNC_SCL5>, > > > + <PINMUX_GPIO49__FUNC_SDA5>; > > > + mediatek,pull-up-adv = <11>; > > > + }; > > > + }; > > > + > > > + i2c1_pins_a: i2c1@0 { > > > + pins { > > > + pinmux = <PINMUX_GPIO50__FUNC_SCL3>, > > > + <PINMUX_GPIO51__FUNC_SDA3>; > > > + mediatek,pull-down-adv = <10>; > > > + }; > > > + }; > > > + ... > > > + }; > > > +}; > > > -- > > > 1.9.1 > > > > > > > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek
On Fri, 2019-01-04 at 01:14 -0800, Sean Wang wrote: > On Thu, Jan 3, 2019 at 11:09 PM Zhiyong Tao <zhiyong.tao@mediatek.com> wrote: > > > > On Sat, 2018-12-29 at 06:04 +0800, Rob Herring wrote: > > > On Fri, Dec 28, 2018 at 04:09:40PM +0800, Erin Lo wrote: > > > > From: Zhiyong Tao <zhiyong.tao@mediatek.com> > > > > > > > > The commit adds mt8183 compatible node in binding document. > > > > > > > > Signed-off-by: Zhiyong Tao <zhiyong.tao@mediatek.com> > > > > Signed-off-by: Erin Lo <erin.lo@mediatek.com> > > > > --- > > > > .../devicetree/bindings/pinctrl/pinctrl-mt8183.txt | 110 +++++++++++++++++++++ > > > > 1 file changed, 110 insertions(+) > > > > create mode 100644 Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt > > > > > > > > diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt > > > > new file mode 100644 > > > > index 0000000..7b5285e > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt > > > > @@ -0,0 +1,110 @@ > > > > +* Mediatek MT8183 Pin Controller > > > > + > > > > +The Mediatek's Pin controller is used to control SoC pins. > > > > + > > > > +Required properties: > > > > +- compatible: value should be one of the following. > > > > + "mediatek,mt8183-pinctrl", compatible with mt8183 pinctrl. > > > > +- gpio-controller : Marks the device node as a gpio controller. > > > > +- #gpio-cells: number of cells in GPIO specifier. Since the generic GPIO > > > > + binding is used, the amount of cells must be specified as 2. See the below > > > > + mentioned gpio binding representation for description of particular cells. > > > > +- gpio-ranges : gpio valid number range. > > > > + > > > > + Eg: <&pio 6 0> > > > > + <[phandle of the gpio controller node] > > > > + [line number within the gpio controller] > > > > + [flags]> > > > > + > > > > + Values for gpio specifier: > > > > + - Line number: is a value between 0 to 202. > > > > + - Flags: bit field of flags, as defined in <dt-bindings/gpio/gpio.h>. > > > > + Only the following flags are supported: > > > > + 0 - GPIO_ACTIVE_HIGH > > > > + 1 - GPIO_ACTIVE_LOW > > > > + > > > > +Optional properties: > > > > +- reg: physicall address base for gpio base registers. > > > > > > s/physicall/physical/ > > > > > > reg should never be optional. > > > > > > Need to say how many reg entries. > > > > ==> Thanks for your suggestion. We will change 'reg' to Required > > properties and add the reg entries in next version. > > > > > > > +- reg-names: gpio base registers name. > > > > > > Need to say what are the names. However, I don't find the names in the > > > example all that useful, so I'd just drop it. > > > > ==> we will add the reg-names in next version. > > They are used in the sample code, such as: > > > + reg-names = "iocfg0", "iocfg1", "iocfg2", > > > > + "iocfg3", "iocfg4", "iocfg5", > > > > + "iocfg6", "iocfg7", "iocfg8"; > > > > > > > > > +- interrupt-controller: Marks the device node as an interrupt controller > > > > +- #interrupt-cells: Should be two. > > > > +- interrupts : The interrupt outputs from the controller. > > > > + > > > > +Please refer to pinctrl-bindings.txt in this directory for details of the > > > > +common pinctrl bindings used by client devices. > > > > + > > > > +Subnode format > > > > +A pinctrl node should contain at least one subnodes representing the > > > > +pinctrl groups available on the machine. Each subnode will list the > > > > +pins it needs, and how they should be configured, with regard to muxer > > > > +configuration, pullups, drive strength, input enable/disable and input schmitt. > > > > + > > > > + node { > > > > + pinmux = <PIN_NUMBER_PINMUX>; > > > > + GENERIC_PINCONFIG; > > > > + }; > > > > + > > > > +Required properties: > > > > +- pinmux: integer array, represents gpio pin number and mux setting. > > > > + Supported pin number and mux varies for different SoCs, and are defined > > > > + as macros in boot/dts/<soc>-pinfunc.h directly. > > > > + > > > > +Optional properties: > > > > +- GENERIC_PINCONFIG: is the generic pinconfig options to use, bias-disable, > > > > + bias-pull-down, bias-pull-up, input-enable, input-disable, output-low, output-high, > > > > + input-schmitt-enable, input-schmitt-disable and drive-strength are valid. > > > > + > > > > + Some special pins have extra pull up strength, there are R0 and R1 pull-up > > > > + resistors available, but for user, it's only need to set R1R0 as 00, 01, 10 or 11. > > > > + So when config mediatek,pull-up-adv or mediatek,pull-down-adv, > > > > + it support arguments for those special pins. > > > > + > > > > + When config drive-strength, it can support some arguments, such as > > > > + MTK_DRIVE_4mA, MTK_DRIVE_6mA, etc. See dt-bindings/pinctrl/mt65xx.h. > > > > + > > One point we can fix more is the drive-strength already is supported > as the generic one, not need to depend on a dedicated header file. ==> We think that we use MTK_DRIVE_4mA to set as driving in dts. it it more clear in v2 common driver for single pin -> single group mode. How about the idea? Thanks. > > > > > +Examples: > > > > + > > > > +#include "mt8183-pinfunc.h" > > > > + > > > > +... > > > > +{ > > > > + pio: pinctrl@10005000 { > > > > + compatible = "mediatek,mt8183-pinctrl"; > > > > + reg = <0 0x10005000 0 0x1000>, > > > > + <0 0x11F20000 0 0x1000>, > > > > + <0 0x11E80000 0 0x1000>, > > > > + <0 0x11E70000 0 0x1000>, > > > > + <0 0x11E90000 0 0x1000>, > > > > + <0 0x11D30000 0 0x1000>, > > > > + <0 0x11D20000 0 0x1000>, > > > > + <0 0x11C50000 0 0x1000>, > > > > + <0 0x11F30000 0 0x1000>; > > > > + reg-names = "iocfg0", "iocfg1", "iocfg2", > > > > + "iocfg3", "iocfg4", "iocfg5", > > > > + "iocfg6", "iocfg7", "iocfg8"; > > > > + gpio-controller; > > > > + #gpio-cells = <2>; > > > > + gpio-ranges = <&pio 0 0 192>; > > > > + interrupt-controller; > > > > + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>; > > > > + interrupt-parent = <&gic>; > > > > + #interrupt-cells = <2>; > > > > + > > > > + i2c0_pins_a: i2c0@0 { > > > > > > unit-address without reg property is not valid. > > > > ==> we will change "i2c0_pins_a: i2c0@0" to "i2c0_pins_a: i2c0" in the > > next version. > > > > > > > + pins1 { > > > > + pinmux = <PINMUX_GPIO48__FUNC_SCL5>, > > > > + <PINMUX_GPIO49__FUNC_SDA5>; > > > > + mediatek,pull-up-adv = <11>; > > > > + }; > > > > + }; > > > > + > > > > + i2c1_pins_a: i2c1@0 { > > > > + pins { > > > > + pinmux = <PINMUX_GPIO50__FUNC_SCL3>, > > > > + <PINMUX_GPIO51__FUNC_SDA3>; > > > > + mediatek,pull-down-adv = <10>; > > > > + }; > > > > + }; > > > > + ... > > > > + }; > > > > +}; > > > > -- > > > > 1.9.1 > > > > > > > > > > > > _______________________________________________ > > Linux-mediatek mailing list > > Linux-mediatek@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-mediatek
On Fri, Jan 4, 2019 at 1:40 AM Zhiyong Tao <zhiyong.tao@mediatek.com> wrote: > > On Fri, 2019-01-04 at 01:14 -0800, Sean Wang wrote: > > On Thu, Jan 3, 2019 at 11:09 PM Zhiyong Tao <zhiyong.tao@mediatek.com> wrote: > > > > > > On Sat, 2018-12-29 at 06:04 +0800, Rob Herring wrote: > > > > On Fri, Dec 28, 2018 at 04:09:40PM +0800, Erin Lo wrote: > > > > > From: Zhiyong Tao <zhiyong.tao@mediatek.com> > > > > > > > > > > The commit adds mt8183 compatible node in binding document. > > > > > > > > > > Signed-off-by: Zhiyong Tao <zhiyong.tao@mediatek.com> > > > > > Signed-off-by: Erin Lo <erin.lo@mediatek.com> > > > > > --- > > > > > .../devicetree/bindings/pinctrl/pinctrl-mt8183.txt | 110 +++++++++++++++++++++ > > > > > 1 file changed, 110 insertions(+) > > > > > create mode 100644 Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt > > > > > new file mode 100644 > > > > > index 0000000..7b5285e > > > > > --- /dev/null > > > > > +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt > > > > > @@ -0,0 +1,110 @@ > > > > > +* Mediatek MT8183 Pin Controller > > > > > + > > > > > +The Mediatek's Pin controller is used to control SoC pins. > > > > > + > > > > > +Required properties: > > > > > +- compatible: value should be one of the following. > > > > > + "mediatek,mt8183-pinctrl", compatible with mt8183 pinctrl. > > > > > +- gpio-controller : Marks the device node as a gpio controller. > > > > > +- #gpio-cells: number of cells in GPIO specifier. Since the generic GPIO > > > > > + binding is used, the amount of cells must be specified as 2. See the below > > > > > + mentioned gpio binding representation for description of particular cells. > > > > > +- gpio-ranges : gpio valid number range. > > > > > + > > > > > + Eg: <&pio 6 0> > > > > > + <[phandle of the gpio controller node] > > > > > + [line number within the gpio controller] > > > > > + [flags]> > > > > > + > > > > > + Values for gpio specifier: > > > > > + - Line number: is a value between 0 to 202. > > > > > + - Flags: bit field of flags, as defined in <dt-bindings/gpio/gpio.h>. > > > > > + Only the following flags are supported: > > > > > + 0 - GPIO_ACTIVE_HIGH > > > > > + 1 - GPIO_ACTIVE_LOW > > > > > + > > > > > +Optional properties: > > > > > +- reg: physicall address base for gpio base registers. > > > > > > > > s/physicall/physical/ > > > > > > > > reg should never be optional. > > > > > > > > Need to say how many reg entries. > > > > > > ==> Thanks for your suggestion. We will change 'reg' to Required > > > properties and add the reg entries in next version. > > > > > > > > > +- reg-names: gpio base registers name. > > > > > > > > Need to say what are the names. However, I don't find the names in the > > > > example all that useful, so I'd just drop it. > > > > > > ==> we will add the reg-names in next version. > > > They are used in the sample code, such as: > > > > + reg-names = "iocfg0", "iocfg1", "iocfg2", > > > > > + "iocfg3", "iocfg4", "iocfg5", > > > > > + "iocfg6", "iocfg7", "iocfg8"; > > > > > > > > > > > > +- interrupt-controller: Marks the device node as an interrupt controller > > > > > +- #interrupt-cells: Should be two. > > > > > +- interrupts : The interrupt outputs from the controller. > > > > > + > > > > > +Please refer to pinctrl-bindings.txt in this directory for details of the > > > > > +common pinctrl bindings used by client devices. > > > > > + > > > > > +Subnode format > > > > > +A pinctrl node should contain at least one subnodes representing the > > > > > +pinctrl groups available on the machine. Each subnode will list the > > > > > +pins it needs, and how they should be configured, with regard to muxer > > > > > +configuration, pullups, drive strength, input enable/disable and input schmitt. > > > > > + > > > > > + node { > > > > > + pinmux = <PIN_NUMBER_PINMUX>; > > > > > + GENERIC_PINCONFIG; > > > > > + }; > > > > > + > > > > > +Required properties: > > > > > +- pinmux: integer array, represents gpio pin number and mux setting. > > > > > + Supported pin number and mux varies for different SoCs, and are defined > > > > > + as macros in boot/dts/<soc>-pinfunc.h directly. > > > > > + > > > > > +Optional properties: > > > > > +- GENERIC_PINCONFIG: is the generic pinconfig options to use, bias-disable, > > > > > + bias-pull-down, bias-pull-up, input-enable, input-disable, output-low, output-high, > > > > > + input-schmitt-enable, input-schmitt-disable and drive-strength are valid. > > > > > + > > > > > + Some special pins have extra pull up strength, there are R0 and R1 pull-up > > > > > + resistors available, but for user, it's only need to set R1R0 as 00, 01, 10 or 11. > > > > > + So when config mediatek,pull-up-adv or mediatek,pull-down-adv, > > > > > + it support arguments for those special pins. > > > > > + > > > > > + When config drive-strength, it can support some arguments, such as > > > > > + MTK_DRIVE_4mA, MTK_DRIVE_6mA, etc. See dt-bindings/pinctrl/mt65xx.h. > > > > > + > > > > One point we can fix more is the drive-strength already is supported > > as the generic one, not need to depend on a dedicated header file. > > ==> We think that we use MTK_DRIVE_4mA to set as driving in dts. it it > more clear in v2 common driver for single pin -> single group mode. > How about the idea? > Sure, thank for post your idea and it sounds not bad to keep the convention for the single pin -> single group mode binding. > Thanks. > > > > > > > +Examples: > > > > > + > > > > > +#include "mt8183-pinfunc.h" > > > > > + > > > > > +... > > > > > +{ > > > > > + pio: pinctrl@10005000 { > > > > > + compatible = "mediatek,mt8183-pinctrl"; > > > > > + reg = <0 0x10005000 0 0x1000>, > > > > > + <0 0x11F20000 0 0x1000>, > > > > > + <0 0x11E80000 0 0x1000>, > > > > > + <0 0x11E70000 0 0x1000>, > > > > > + <0 0x11E90000 0 0x1000>, > > > > > + <0 0x11D30000 0 0x1000>, > > > > > + <0 0x11D20000 0 0x1000>, > > > > > + <0 0x11C50000 0 0x1000>, > > > > > + <0 0x11F30000 0 0x1000>; > > > > > + reg-names = "iocfg0", "iocfg1", "iocfg2", > > > > > + "iocfg3", "iocfg4", "iocfg5", > > > > > + "iocfg6", "iocfg7", "iocfg8"; > > > > > + gpio-controller; > > > > > + #gpio-cells = <2>; > > > > > + gpio-ranges = <&pio 0 0 192>; > > > > > + interrupt-controller; > > > > > + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>; > > > > > + interrupt-parent = <&gic>; > > > > > + #interrupt-cells = <2>; > > > > > + > > > > > + i2c0_pins_a: i2c0@0 { > > > > > > > > unit-address without reg property is not valid. > > > > > > ==> we will change "i2c0_pins_a: i2c0@0" to "i2c0_pins_a: i2c0" in the > > > next version. > > > > > > > > > + pins1 { > > > > > + pinmux = <PINMUX_GPIO48__FUNC_SCL5>, > > > > > + <PINMUX_GPIO49__FUNC_SDA5>; > > > > > + mediatek,pull-up-adv = <11>; > > > > > + }; > > > > > + }; > > > > > + > > > > > + i2c1_pins_a: i2c1@0 { > > > > > + pins { > > > > > + pinmux = <PINMUX_GPIO50__FUNC_SCL3>, > > > > > + <PINMUX_GPIO51__FUNC_SDA3>; > > > > > + mediatek,pull-down-adv = <10>; > > > > > + }; > > > > > + }; > > > > > + ... > > > > > + }; > > > > > +}; > > > > > -- > > > > > 1.9.1 > > > > > > > > > > > > > > > > > _______________________________________________ > > > Linux-mediatek mailing list > > > Linux-mediatek@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-mediatek > >
On Fri, 2019-01-04 at 01:55 -0800, Sean Wang wrote: > On Fri, Jan 4, 2019 at 1:40 AM Zhiyong Tao <zhiyong.tao@mediatek.com> wrote: > > > > On Fri, 2019-01-04 at 01:14 -0800, Sean Wang wrote: > > > On Thu, Jan 3, 2019 at 11:09 PM Zhiyong Tao <zhiyong.tao@mediatek.com> wrote: > > > > > > > > On Sat, 2018-12-29 at 06:04 +0800, Rob Herring wrote: > > > > > On Fri, Dec 28, 2018 at 04:09:40PM +0800, Erin Lo wrote: > > > > > > From: Zhiyong Tao <zhiyong.tao@mediatek.com> > > > > > > > > > > > > The commit adds mt8183 compatible node in binding document. > > > > > > > > > > > > Signed-off-by: Zhiyong Tao <zhiyong.tao@mediatek.com> > > > > > > Signed-off-by: Erin Lo <erin.lo@mediatek.com> > > > > > > --- > > > > > > .../devicetree/bindings/pinctrl/pinctrl-mt8183.txt | 110 +++++++++++++++++++++ > > > > > > 1 file changed, 110 insertions(+) > > > > > > create mode 100644 Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt > > > > > > new file mode 100644 > > > > > > index 0000000..7b5285e > > > > > > --- /dev/null > > > > > > +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt > > > > > > @@ -0,0 +1,110 @@ > > > > > > +* Mediatek MT8183 Pin Controller > > > > > > + > > > > > > +The Mediatek's Pin controller is used to control SoC pins. > > > > > > + > > > > > > +Required properties: > > > > > > +- compatible: value should be one of the following. > > > > > > + "mediatek,mt8183-pinctrl", compatible with mt8183 pinctrl. > > > > > > +- gpio-controller : Marks the device node as a gpio controller. > > > > > > +- #gpio-cells: number of cells in GPIO specifier. Since the generic GPIO > > > > > > + binding is used, the amount of cells must be specified as 2. See the below > > > > > > + mentioned gpio binding representation for description of particular cells. > > > > > > +- gpio-ranges : gpio valid number range. > > > > > > + > > > > > > + Eg: <&pio 6 0> > > > > > > + <[phandle of the gpio controller node] > > > > > > + [line number within the gpio controller] > > > > > > + [flags]> > > > > > > + > > > > > > + Values for gpio specifier: > > > > > > + - Line number: is a value between 0 to 202. > > > > > > + - Flags: bit field of flags, as defined in <dt-bindings/gpio/gpio.h>. > > > > > > + Only the following flags are supported: > > > > > > + 0 - GPIO_ACTIVE_HIGH > > > > > > + 1 - GPIO_ACTIVE_LOW > > > > > > + > > > > > > +Optional properties: > > > > > > +- reg: physicall address base for gpio base registers. > > > > > > > > > > s/physicall/physical/ > > > > > > > > > > reg should never be optional. > > > > > > > > > > Need to say how many reg entries. > > > > > > > > ==> Thanks for your suggestion. We will change 'reg' to Required > > > > properties and add the reg entries in next version. > > > > > > > > > > > +- reg-names: gpio base registers name. > > > > > > > > > > Need to say what are the names. However, I don't find the names in the > > > > > example all that useful, so I'd just drop it. > > > > > > > > ==> we will add the reg-names in next version. > > > > They are used in the sample code, such as: > > > > > + reg-names = "iocfg0", "iocfg1", "iocfg2", > > > > > > + "iocfg3", "iocfg4", "iocfg5", > > > > > > + "iocfg6", "iocfg7", "iocfg8"; > > > > > > > > > > > > > > > +- interrupt-controller: Marks the device node as an interrupt controller > > > > > > +- #interrupt-cells: Should be two. > > > > > > +- interrupts : The interrupt outputs from the controller. > > > > > > + > > > > > > +Please refer to pinctrl-bindings.txt in this directory for details of the > > > > > > +common pinctrl bindings used by client devices. > > > > > > + > > > > > > +Subnode format > > > > > > +A pinctrl node should contain at least one subnodes representing the > > > > > > +pinctrl groups available on the machine. Each subnode will list the > > > > > > +pins it needs, and how they should be configured, with regard to muxer > > > > > > +configuration, pullups, drive strength, input enable/disable and input schmitt. > > > > > > + > > > > > > + node { > > > > > > + pinmux = <PIN_NUMBER_PINMUX>; > > > > > > + GENERIC_PINCONFIG; > > > > > > + }; > > > > > > + > > > > > > +Required properties: > > > > > > +- pinmux: integer array, represents gpio pin number and mux setting. > > > > > > + Supported pin number and mux varies for different SoCs, and are defined > > > > > > + as macros in boot/dts/<soc>-pinfunc.h directly. > > > > > > + > > > > > > +Optional properties: > > > > > > +- GENERIC_PINCONFIG: is the generic pinconfig options to use, bias-disable, > > > > > > + bias-pull-down, bias-pull-up, input-enable, input-disable, output-low, output-high, > > > > > > + input-schmitt-enable, input-schmitt-disable and drive-strength are valid. > > > > > > + > > > > > > + Some special pins have extra pull up strength, there are R0 and R1 pull-up > > > > > > + resistors available, but for user, it's only need to set R1R0 as 00, 01, 10 or 11. > > > > > > + So when config mediatek,pull-up-adv or mediatek,pull-down-adv, > > > > > > + it support arguments for those special pins. > > > > > > + > > > > > > + When config drive-strength, it can support some arguments, such as > > > > > > + MTK_DRIVE_4mA, MTK_DRIVE_6mA, etc. See dt-bindings/pinctrl/mt65xx.h. > > > > > > + > > > > > > One point we can fix more is the drive-strength already is supported > > > as the generic one, not need to depend on a dedicated header file. > > > > ==> We think that we use MTK_DRIVE_4mA to set as driving in dts. it it > > more clear in v2 common driver for single pin -> single group mode. > > How about the idea? > > > > Sure, thank for post your idea and it sounds not bad to keep the > convention for the single pin -> single group mode binding. Thanks for your confirm. We will keep it in next version. > > > Thanks. > > > > > > > > > +Examples: > > > > > > + > > > > > > +#include "mt8183-pinfunc.h" > > > > > > + > > > > > > +... > > > > > > +{ > > > > > > + pio: pinctrl@10005000 { > > > > > > + compatible = "mediatek,mt8183-pinctrl"; > > > > > > + reg = <0 0x10005000 0 0x1000>, > > > > > > + <0 0x11F20000 0 0x1000>, > > > > > > + <0 0x11E80000 0 0x1000>, > > > > > > + <0 0x11E70000 0 0x1000>, > > > > > > + <0 0x11E90000 0 0x1000>, > > > > > > + <0 0x11D30000 0 0x1000>, > > > > > > + <0 0x11D20000 0 0x1000>, > > > > > > + <0 0x11C50000 0 0x1000>, > > > > > > + <0 0x11F30000 0 0x1000>; > > > > > > + reg-names = "iocfg0", "iocfg1", "iocfg2", > > > > > > + "iocfg3", "iocfg4", "iocfg5", > > > > > > + "iocfg6", "iocfg7", "iocfg8"; > > > > > > + gpio-controller; > > > > > > + #gpio-cells = <2>; > > > > > > + gpio-ranges = <&pio 0 0 192>; > > > > > > + interrupt-controller; > > > > > > + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>; > > > > > > + interrupt-parent = <&gic>; > > > > > > + #interrupt-cells = <2>; > > > > > > + > > > > > > + i2c0_pins_a: i2c0@0 { > > > > > > > > > > unit-address without reg property is not valid. > > > > > > > > ==> we will change "i2c0_pins_a: i2c0@0" to "i2c0_pins_a: i2c0" in the > > > > next version. > > > > > > > > > > > + pins1 { > > > > > > + pinmux = <PINMUX_GPIO48__FUNC_SCL5>, > > > > > > + <PINMUX_GPIO49__FUNC_SDA5>; > > > > > > + mediatek,pull-up-adv = <11>; > > > > > > + }; > > > > > > + }; > > > > > > + > > > > > > + i2c1_pins_a: i2c1@0 { > > > > > > + pins { > > > > > > + pinmux = <PINMUX_GPIO50__FUNC_SCL3>, > > > > > > + <PINMUX_GPIO51__FUNC_SDA3>; > > > > > > + mediatek,pull-down-adv = <10>; > > > > > > + }; > > > > > > + }; > > > > > > + ... > > > > > > + }; > > > > > > +}; > > > > > > -- > > > > > > 1.9.1 > > > > > > > > > > > > > > > > > > > > > > _______________________________________________ > > > > Linux-mediatek mailing list > > > > Linux-mediatek@lists.infradead.org > > > > http://lists.infradead.org/mailman/listinfo/linux-mediatek > > > >
diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt new file mode 100644 index 0000000..7b5285e --- /dev/null +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8183.txt @@ -0,0 +1,110 @@ +* Mediatek MT8183 Pin Controller + +The Mediatek's Pin controller is used to control SoC pins. + +Required properties: +- compatible: value should be one of the following. + "mediatek,mt8183-pinctrl", compatible with mt8183 pinctrl. +- gpio-controller : Marks the device node as a gpio controller. +- #gpio-cells: number of cells in GPIO specifier. Since the generic GPIO + binding is used, the amount of cells must be specified as 2. See the below + mentioned gpio binding representation for description of particular cells. +- gpio-ranges : gpio valid number range. + + Eg: <&pio 6 0> + <[phandle of the gpio controller node] + [line number within the gpio controller] + [flags]> + + Values for gpio specifier: + - Line number: is a value between 0 to 202. + - Flags: bit field of flags, as defined in <dt-bindings/gpio/gpio.h>. + Only the following flags are supported: + 0 - GPIO_ACTIVE_HIGH + 1 - GPIO_ACTIVE_LOW + +Optional properties: +- reg: physicall address base for gpio base registers. +- reg-names: gpio base registers name. +- interrupt-controller: Marks the device node as an interrupt controller +- #interrupt-cells: Should be two. +- interrupts : The interrupt outputs from the controller. + +Please refer to pinctrl-bindings.txt in this directory for details of the +common pinctrl bindings used by client devices. + +Subnode format +A pinctrl node should contain at least one subnodes representing the +pinctrl groups available on the machine. Each subnode will list the +pins it needs, and how they should be configured, with regard to muxer +configuration, pullups, drive strength, input enable/disable and input schmitt. + + node { + pinmux = <PIN_NUMBER_PINMUX>; + GENERIC_PINCONFIG; + }; + +Required properties: +- pinmux: integer array, represents gpio pin number and mux setting. + Supported pin number and mux varies for different SoCs, and are defined + as macros in boot/dts/<soc>-pinfunc.h directly. + +Optional properties: +- GENERIC_PINCONFIG: is the generic pinconfig options to use, bias-disable, + bias-pull-down, bias-pull-up, input-enable, input-disable, output-low, output-high, + input-schmitt-enable, input-schmitt-disable and drive-strength are valid. + + Some special pins have extra pull up strength, there are R0 and R1 pull-up + resistors available, but for user, it's only need to set R1R0 as 00, 01, 10 or 11. + So when config mediatek,pull-up-adv or mediatek,pull-down-adv, + it support arguments for those special pins. + + When config drive-strength, it can support some arguments, such as + MTK_DRIVE_4mA, MTK_DRIVE_6mA, etc. See dt-bindings/pinctrl/mt65xx.h. + +Examples: + +#include "mt8183-pinfunc.h" + +... +{ + pio: pinctrl@10005000 { + compatible = "mediatek,mt8183-pinctrl"; + reg = <0 0x10005000 0 0x1000>, + <0 0x11F20000 0 0x1000>, + <0 0x11E80000 0 0x1000>, + <0 0x11E70000 0 0x1000>, + <0 0x11E90000 0 0x1000>, + <0 0x11D30000 0 0x1000>, + <0 0x11D20000 0 0x1000>, + <0 0x11C50000 0 0x1000>, + <0 0x11F30000 0 0x1000>; + reg-names = "iocfg0", "iocfg1", "iocfg2", + "iocfg3", "iocfg4", "iocfg5", + "iocfg6", "iocfg7", "iocfg8"; + gpio-controller; + #gpio-cells = <2>; + gpio-ranges = <&pio 0 0 192>; + interrupt-controller; + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>; + interrupt-parent = <&gic>; + #interrupt-cells = <2>; + + i2c0_pins_a: i2c0@0 { + pins1 { + pinmux = <PINMUX_GPIO48__FUNC_SCL5>, + <PINMUX_GPIO49__FUNC_SDA5>; + mediatek,pull-up-adv = <11>; + }; + }; + + i2c1_pins_a: i2c1@0 { + pins { + pinmux = <PINMUX_GPIO50__FUNC_SCL3>, + <PINMUX_GPIO51__FUNC_SDA3>; + mediatek,pull-down-adv = <10>; + }; + }; + ... + }; +};