| Message ID | 1535634775-19365-1-git-send-email-phil.edworthy@renesas.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Renesas R9A06G032 PINCTRL Driver | expand |
Hi Phil, On Thu, Aug 30, 2018 at 02:12:52PM +0100, Phil Edworthy wrote: > This implements the pinctrl driver for the RZ/N1 family of devices, including > the R9A06G032 (RZ/N1D) device. > > One area that is likely to be contentious is the use of 'virtual pins' for the > MDIO pinmuxing. The driver uses two pins (170 and 171) that don't exist on the > device to configure the MDIO source within the RZ/N1 devices. On these devices, > there are two Ethernet MACs, a 5-Port Switch, numerous industrial Ethernet > peripherals, any of which can be the MDIO source. Configuring the MDIO source > could be done without the virtual pins, e.g. by extending the functions to > cover all MDIO variants (a total of 32 additional functions), but this would > allow users to misconfigure individual MDIO pins, rather than assign all MDIO > pins to a MDIO source. The choice of how to implement this will affect the > DT bindings. > > This series was originally written by Michel Pollet whilst at Renesas, and I > have taken over this work. > > One point from Michel's v1 series: > "Note, I used renesas,rzn1-pinmux node to specify the pinmux constants, > and I also don't use some of the properties documented in > pinctrl-bindings.txt on purpose, as they are too limited for my use > (I need to be able to set, clear, ignore or reset level, pull up/down > and function as the pinmux might be set by another OS/core running > concurently)." > I start by saying that I don't know this HW pin controller well, so I might be missing something, but as commented on the original series from Micheal, I still don't see why you need a custom property here... My understanding, looking at this comment and the header provided by patch [1/3] (include/dt-bindings/pinctrl/rzn1-pinctrl.h) is that basically need to control pull-up/down and the output driver strength. Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt reports a set of generic pin configuration properties to be applied to a pin configuration (and multiplexing) pin controller child node that fully express all (most?) of your needs. Eg. a pin configuration with pull up applied, using examples from your cover letter should be expressed as Your example: &pinctrl { pinsuart0: pinsuart0 { renesas,rzn1-pinmux-ids = < RZN1_MUX(103, UART0_I) /* UART0_TXD */ RZN1_MUX_PUP(104, UART0_I) /* UART0_RXD */ >; }; }; Using standard pinctroller bindings pin configuration properties: &pinctrl { pinsuart0: uart0 { pinsuart_tx0 { pinmux = <103, UART0_I>; /* UART0_TXD */ }; pinsuart_rx0 { pinmux = <104, UART0_I>; /* UART0_RXD */ bias-pull-up; }; }; }; Is there anything I am missing? Maybe from the interaction with "another OS/core running concurrently" you mentioned? In this case if you only have to perform pin configuration (because muxing is handled already) things are even simpler, just use the pin configuration bindings, without involving muxing at all: &pinctrl { pinsuart_conf: uart0 { pins = <103, 104>; bias-pull-up; }; }; Thanks j > Patch 0003 should really be applied after patch: > "ARM: dts: r9a06g032: Correct UART and add all other UARTs", see > https://www.spinics.net/lists/arm-kernel/msg673525.html > > Main changes: > v2: > - Change to generic rzn1 family driver, instead of device specific. > - Review comments fixed. > - Fix error handling during probe > > Phil Edworthy (3): > dt-bindings: pinctrl: renesas,rzn1-pinctrl: documentation > pinctrl: renesas: Renesas RZ/N1 pinctrl driver > ARM: dts: r9a06g032: Add pinctrl node > > .../bindings/pinctrl/renesas,rzn1-pinctrl.txt | 97 +++ > arch/arm/boot/dts/r9a06g032.dtsi | 8 + > drivers/pinctrl/Kconfig | 10 + > drivers/pinctrl/Makefile | 1 + > drivers/pinctrl/pinctrl-rzn1.c | 844 +++++++++++++++++++++ > include/dt-bindings/pinctrl/rzn1-pinctrl.h | 191 +++++ > 6 files changed, 1151 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pinctrl/renesas,rzn1-pinctrl.txt > create mode 100644 drivers/pinctrl/pinctrl-rzn1.c > create mode 100644 include/dt-bindings/pinctrl/rzn1-pinctrl.h > > -- > 2.7.4 >
Hi Jacopo, On 03 September 2018 11:34, jacopo mondi wrote: > On Thu, Aug 30, 2018 at 02:12:52PM +0100, Phil Edworthy wrote: > > This implements the pinctrl driver for the RZ/N1 family of devices, including > > the R9A06G032 (RZ/N1D) device. > > > > One area that is likely to be contentious is the use of 'virtual pins' for the > > MDIO pinmuxing. The driver uses two pins (170 and 171) that don't exist on > the > > device to configure the MDIO source within the RZ/N1 devices. On these > devices, > > there are two Ethernet MACs, a 5-Port Switch, numerous industrial > Ethernet > > peripherals, any of which can be the MDIO source. Configuring the MDIO > source > > could be done without the virtual pins, e.g. by extending the functions to > > cover all MDIO variants (a total of 32 additional functions), but this would > > allow users to misconfigure individual MDIO pins, rather than assign all > MDIO > > pins to a MDIO source. The choice of how to implement this will affect the > > DT bindings. > > > > This series was originally written by Michel Pollet whilst at Renesas, and I > > have taken over this work. > > > > One point from Michel's v1 series: > > "Note, I used renesas,rzn1-pinmux node to specify the pinmux constants, > > and I also don't use some of the properties documented in > > pinctrl-bindings.txt on purpose, as they are too limited for my use > > (I need to be able to set, clear, ignore or reset level, pull up/down > > and function as the pinmux might be set by another OS/core running > > concurently)." > > > > I start by saying that I don't know this HW pin controller well, so > I might be missing something, but as commented on the original series from > Micheal, I still don't see why you need a custom property here... > > My understanding, looking at this comment and the header provided by > patch [1/3] (include/dt-bindings/pinctrl/rzn1-pinctrl.h) is that > basically need to control pull-up/down and the output driver strength. > > Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt reports > a set of generic pin configuration properties to be applied to a pin > configuration (and multiplexing) pin controller child node that fully > express all (most?) of your needs. > > Eg. a pin configuration with pull up applied, using examples from your > cover letter should be expressed as > > Your example: > &pinctrl { > pinsuart0: pinsuart0 { > renesas,rzn1-pinmux-ids = < > RZN1_MUX(103, UART0_I) /* UART0_TXD */ > RZN1_MUX_PUP(104, UART0_I) /* UART0_RXD */ > >; > }; > }; > > Using standard pinctroller bindings pin configuration properties: > > &pinctrl { > pinsuart0: uart0 { > pinsuart_tx0 { > pinmux = <103, UART0_I>; /* UART0_TXD */ > }; > > pinsuart_rx0 { > pinmux = <104, UART0_I>; /* UART0_RXD */ > bias-pull-up; > }; > }; > }; > > Is there anything I am missing? Maybe from the interaction with > "another OS/core running concurrently" you mentioned? In this case if > you only have to perform pin configuration (because muxing is handled > already) things are even simpler, just use the pin configuration > bindings, without involving muxing at all: > > &pinctrl { > pinsuart_conf: uart0 { > pins = <103, 104>; > bias-pull-up; > }; > }; Sorry I didn’t address your point. The only reason we want to use new properties is so the driver can process dts files that have been generated from an existing PinMux App. That output is used by VxWorks as well as our out-of-tree Linux port. If that is not a good enough reason to add new properties, then I can't see any technical reason not to use the existing bindings. The use with another OS running on a different core should not be a barrier as it must not use the same pins as Linux. Thanks Phil > Thanks > j > > > Patch 0003 should really be applied after patch: > > "ARM: dts: r9a06g032: Correct UART and add all other UARTs", see > > https://www.spinics.net/lists/arm-kernel/msg673525.html > > > > Main changes: > > v2: > > - Change to generic rzn1 family driver, instead of device specific. > > - Review comments fixed. > > - Fix error handling during probe > > > > Phil Edworthy (3): > > dt-bindings: pinctrl: renesas,rzn1-pinctrl: documentation > > pinctrl: renesas: Renesas RZ/N1 pinctrl driver > > ARM: dts: r9a06g032: Add pinctrl node > > > > .../bindings/pinctrl/renesas,rzn1-pinctrl.txt | 97 +++ > > arch/arm/boot/dts/r9a06g032.dtsi | 8 + > > drivers/pinctrl/Kconfig | 10 + > > drivers/pinctrl/Makefile | 1 + > > drivers/pinctrl/pinctrl-rzn1.c | 844 +++++++++++++++++++++ > > include/dt-bindings/pinctrl/rzn1-pinctrl.h | 191 +++++ > > 6 files changed, 1151 insertions(+) > > create mode 100644 > Documentation/devicetree/bindings/pinctrl/renesas,rzn1-pinctrl.txt > > create mode 100644 drivers/pinctrl/pinctrl-rzn1.c > > create mode 100644 include/dt-bindings/pinctrl/rzn1-pinctrl.h > > > > -- > > 2.7.4 > >
Hi Phil, On Mon, Sep 03, 2018 at 11:03:18AM +0000, Phil Edworthy wrote: > Hi Jacopo, > > On 03 September 2018 11:34, jacopo mondi wrote: > > On Thu, Aug 30, 2018 at 02:12:52PM +0100, Phil Edworthy wrote: > > > This implements the pinctrl driver for the RZ/N1 family of devices, including > > > the R9A06G032 (RZ/N1D) device. > > > > > > One area that is likely to be contentious is the use of 'virtual pins' for the > > > MDIO pinmuxing. The driver uses two pins (170 and 171) that don't exist on > > the > > > device to configure the MDIO source within the RZ/N1 devices. On these > > devices, > > > there are two Ethernet MACs, a 5-Port Switch, numerous industrial > > Ethernet > > > peripherals, any of which can be the MDIO source. Configuring the MDIO > > source > > > could be done without the virtual pins, e.g. by extending the functions to > > > cover all MDIO variants (a total of 32 additional functions), but this would > > > allow users to misconfigure individual MDIO pins, rather than assign all > > MDIO > > > pins to a MDIO source. The choice of how to implement this will affect the > > > DT bindings. > > > > > > This series was originally written by Michel Pollet whilst at Renesas, and I > > > have taken over this work. > > > > > > One point from Michel's v1 series: > > > "Note, I used renesas,rzn1-pinmux node to specify the pinmux constants, > > > and I also don't use some of the properties documented in > > > pinctrl-bindings.txt on purpose, as they are too limited for my use > > > (I need to be able to set, clear, ignore or reset level, pull up/down > > > and function as the pinmux might be set by another OS/core running > > > concurently)." > > > > > > > I start by saying that I don't know this HW pin controller well, so > > I might be missing something, but as commented on the original series from > > Micheal, I still don't see why you need a custom property here... > > > > My understanding, looking at this comment and the header provided by > > patch [1/3] (include/dt-bindings/pinctrl/rzn1-pinctrl.h) is that > > basically need to control pull-up/down and the output driver strength. > > > > Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt reports > > a set of generic pin configuration properties to be applied to a pin > > configuration (and multiplexing) pin controller child node that fully > > express all (most?) of your needs. > > > > Eg. a pin configuration with pull up applied, using examples from your > > cover letter should be expressed as > > > > Your example: > > &pinctrl { > > pinsuart0: pinsuart0 { > > renesas,rzn1-pinmux-ids = < > > RZN1_MUX(103, UART0_I) /* UART0_TXD */ > > RZN1_MUX_PUP(104, UART0_I) /* UART0_RXD */ > > >; > > }; > > }; > > > > Using standard pinctroller bindings pin configuration properties: > > > > &pinctrl { > > pinsuart0: uart0 { > > pinsuart_tx0 { > > pinmux = <103, UART0_I>; /* UART0_TXD */ > > }; > > > > pinsuart_rx0 { > > pinmux = <104, UART0_I>; /* UART0_RXD */ > > bias-pull-up; > > }; > > }; > > }; > > > > Is there anything I am missing? Maybe from the interaction with > > "another OS/core running concurrently" you mentioned? In this case if > > you only have to perform pin configuration (because muxing is handled > > already) things are even simpler, just use the pin configuration > > bindings, without involving muxing at all: > > > > &pinctrl { > > pinsuart_conf: uart0 { > > pins = <103, 104>; > > bias-pull-up; > > }; > > }; > > Sorry I didn’t address your point. > The only reason we want to use new properties is so the driver can process > dts files that have been generated from an existing PinMux App. That output > is used by VxWorks as well as our out-of-tree Linux port. If that is not a > good enough reason to add new properties, then I can't see any technical > reason not to use the existing bindings. I see. I step back then and let this to be handled by the pinctrl subsystem people and maintainer :) Thanks j > The use with another OS running on a different core should not be a barrier > as it must not use the same pins as Linux. > > Thanks > Phil > > > Thanks > > j > > > > > Patch 0003 should really be applied after patch: > > > "ARM: dts: r9a06g032: Correct UART and add all other UARTs", see > > > https://www.spinics.net/lists/arm-kernel/msg673525.html > > > > > > Main changes: > > > v2: > > > - Change to generic rzn1 family driver, instead of device specific. > > > - Review comments fixed. > > > - Fix error handling during probe > > > > > > Phil Edworthy (3): > > > dt-bindings: pinctrl: renesas,rzn1-pinctrl: documentation > > > pinctrl: renesas: Renesas RZ/N1 pinctrl driver > > > ARM: dts: r9a06g032: Add pinctrl node > > > > > > .../bindings/pinctrl/renesas,rzn1-pinctrl.txt | 97 +++ > > > arch/arm/boot/dts/r9a06g032.dtsi | 8 + > > > drivers/pinctrl/Kconfig | 10 + > > > drivers/pinctrl/Makefile | 1 + > > > drivers/pinctrl/pinctrl-rzn1.c | 844 +++++++++++++++++++++ > > > include/dt-bindings/pinctrl/rzn1-pinctrl.h | 191 +++++ > > > 6 files changed, 1151 insertions(+) > > > create mode 100644 > > Documentation/devicetree/bindings/pinctrl/renesas,rzn1-pinctrl.txt > > > create mode 100644 drivers/pinctrl/pinctrl-rzn1.c > > > create mode 100644 include/dt-bindings/pinctrl/rzn1-pinctrl.h > > > > > > -- > > > 2.7.4 > > >
Hi Phil, On Mon, Sep 3, 2018 at 1:03 PM Phil Edworthy <phil.edworthy@renesas.com> wrote: > On 03 September 2018 11:34, jacopo mondi wrote: > > On Thu, Aug 30, 2018 at 02:12:52PM +0100, Phil Edworthy wrote: > > > This implements the pinctrl driver for the RZ/N1 family of devices, including > > > the R9A06G032 (RZ/N1D) device. > > > > > > One area that is likely to be contentious is the use of 'virtual pins' for the > > > MDIO pinmuxing. The driver uses two pins (170 and 171) that don't exist on > > the > > > device to configure the MDIO source within the RZ/N1 devices. On these > > devices, > > > there are two Ethernet MACs, a 5-Port Switch, numerous industrial > > Ethernet > > > peripherals, any of which can be the MDIO source. Configuring the MDIO > > source > > > could be done without the virtual pins, e.g. by extending the functions to > > > cover all MDIO variants (a total of 32 additional functions), but this would > > > allow users to misconfigure individual MDIO pins, rather than assign all > > MDIO > > > pins to a MDIO source. The choice of how to implement this will affect the > > > DT bindings. > > > > > > This series was originally written by Michel Pollet whilst at Renesas, and I > > > have taken over this work. > > > > > > One point from Michel's v1 series: > > > "Note, I used renesas,rzn1-pinmux node to specify the pinmux constants, > > > and I also don't use some of the properties documented in > > > pinctrl-bindings.txt on purpose, as they are too limited for my use > > > (I need to be able to set, clear, ignore or reset level, pull up/down > > > and function as the pinmux might be set by another OS/core running > > > concurently)." > > > > > > > I start by saying that I don't know this HW pin controller well, so > > I might be missing something, but as commented on the original series from > > Micheal, I still don't see why you need a custom property here... > > > > My understanding, looking at this comment and the header provided by > > patch [1/3] (include/dt-bindings/pinctrl/rzn1-pinctrl.h) is that > > basically need to control pull-up/down and the output driver strength. > > > > Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt reports > > a set of generic pin configuration properties to be applied to a pin > > configuration (and multiplexing) pin controller child node that fully > > express all (most?) of your needs. > > > > Eg. a pin configuration with pull up applied, using examples from your > > cover letter should be expressed as > > > > Your example: > > &pinctrl { > > pinsuart0: pinsuart0 { > > renesas,rzn1-pinmux-ids = < > > RZN1_MUX(103, UART0_I) /* UART0_TXD */ > > RZN1_MUX_PUP(104, UART0_I) /* UART0_RXD */ > > >; > > }; > > }; > > > > Using standard pinctroller bindings pin configuration properties: > > > > &pinctrl { > > pinsuart0: uart0 { > > pinsuart_tx0 { > > pinmux = <103, UART0_I>; /* UART0_TXD */ > > }; > > > > pinsuart_rx0 { > > pinmux = <104, UART0_I>; /* UART0_RXD */ > > bias-pull-up; > > }; > > }; > > }; > > > > Is there anything I am missing? Maybe from the interaction with > > "another OS/core running concurrently" you mentioned? In this case if > > you only have to perform pin configuration (because muxing is handled > > already) things are even simpler, just use the pin configuration > > bindings, without involving muxing at all: > > > > &pinctrl { > > pinsuart_conf: uart0 { > > pins = <103, 104>; > > bias-pull-up; > > }; > > }; > > Sorry I didn’t address your point. > The only reason we want to use new properties is so the driver can process > dts files that have been generated from an existing PinMux App. That output > is used by VxWorks as well as our out-of-tree Linux port. If that is not a > good enough reason to add new properties, then I can't see any technical > reason not to use the existing bindings. > The use with another OS running on a different core should not be a barrier > as it must not use the same pins as Linux. Have the VxWorks DT bindings been submitted for review to the devicetree mailing list? Gr{oetje,eeting}s, Geert
Hi Geert, On 05 September 2018 10:37, Geert Uytterhoeven wrote: > On Mon, Sep 3, 2018 at 1:03 PM Phil Edworthy wrote: > > On 03 September 2018 11:34, jacopo mondi wrote: > > > On Thu, Aug 30, 2018 at 02:12:52PM +0100, Phil Edworthy wrote: > > > > This implements the pinctrl driver for the RZ/N1 family of > > > > devices, including the R9A06G032 (RZ/N1D) device. > > > > > > > > One area that is likely to be contentious is the use of 'virtual > > > > pins' for the MDIO pinmuxing. The driver uses two pins (170 and > > > > 171) that don't exist on > > > the > > > > device to configure the MDIO source within the RZ/N1 devices. On > > > > these > > > devices, > > > > there are two Ethernet MACs, a 5-Port Switch, numerous industrial > > > Ethernet > > > > peripherals, any of which can be the MDIO source. Configuring the > > > > MDIO > > > source > > > > could be done without the virtual pins, e.g. by extending the > > > > functions to cover all MDIO variants (a total of 32 additional > > > > functions), but this would allow users to misconfigure individual > > > > MDIO pins, rather than assign all > > > MDIO > > > > pins to a MDIO source. The choice of how to implement this will > > > > affect the DT bindings. > > > > > > > > This series was originally written by Michel Pollet whilst at > > > > Renesas, and I have taken over this work. > > > > > > > > One point from Michel's v1 series: > > > > "Note, I used renesas,rzn1-pinmux node to specify the pinmux > > > > constants, and I also don't use some of the properties documented > > > > in pinctrl-bindings.txt on purpose, as they are too limited for my > > > > use (I need to be able to set, clear, ignore or reset level, pull > > > > up/down and function as the pinmux might be set by another OS/core > > > > running concurently)." > > > > > > > > > > I start by saying that I don't know this HW pin controller well, so > > > I might be missing something, but as commented on the original > > > series from Micheal, I still don't see why you need a custom property > here... > > > > > > My understanding, looking at this comment and the header provided by > > > patch [1/3] (include/dt-bindings/pinctrl/rzn1-pinctrl.h) is that > > > basically need to control pull-up/down and the output driver strength. > > > > > > Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt > > > reports a set of generic pin configuration properties to be applied > > > to a pin configuration (and multiplexing) pin controller child node > > > that fully express all (most?) of your needs. > > > > > > Eg. a pin configuration with pull up applied, using examples from > > > your cover letter should be expressed as > > > > > > Your example: > > > &pinctrl { > > > pinsuart0: pinsuart0 { > > > renesas,rzn1-pinmux-ids = < > > > RZN1_MUX(103, UART0_I) /* UART0_TXD */ > > > RZN1_MUX_PUP(104, UART0_I) /* UART0_RXD */ > > > >; > > > }; > > > }; > > > > > > Using standard pinctroller bindings pin configuration properties: > > > > > > &pinctrl { > > > pinsuart0: uart0 { > > > pinsuart_tx0 { > > > pinmux = <103, UART0_I>; /* UART0_TXD */ > > > }; > > > > > > pinsuart_rx0 { > > > pinmux = <104, UART0_I>; /* UART0_RXD */ > > > bias-pull-up; > > > }; > > > }; > > > }; > > > > > > Is there anything I am missing? Maybe from the interaction with > > > "another OS/core running concurrently" you mentioned? In this case > > > if you only have to perform pin configuration (because muxing is > > > handled > > > already) things are even simpler, just use the pin configuration > > > bindings, without involving muxing at all: > > > > > > &pinctrl { > > > pinsuart_conf: uart0 { > > > pins = <103, 104>; > > > bias-pull-up; > > > }; > > > }; > > > > Sorry I didn’t address your point. > > The only reason we want to use new properties is so the driver can > > process dts files that have been generated from an existing PinMux > > App. That output is used by VxWorks as well as our out-of-tree Linux > > port. If that is not a good enough reason to add new properties, then > > I can't see any technical reason not to use the existing bindings. > > The use with another OS running on a different core should not be a > > barrier as it must not use the same pins as Linux. > > Have the VxWorks DT bindings been submitted for review to the devicetree > mailing list? I'm not involved with the VxWorks port, but I am pretty sure that they have not been submitted for review. Thanks Phil
On Mon, Sep 3, 2018 at 1:25 PM jacopo mondi <jacopo@jmondi.org> wrote: > On Mon, Sep 03, 2018 at 11:03:18AM +0000, Phil Edworthy wrote: > > On 03 September 2018 11:34, jacopo mondi wrote: > > > On Thu, Aug 30, 2018 at 02:12:52PM +0100, Phil Edworthy wrote: > > > > This implements the pinctrl driver for the RZ/N1 family of devices, including > > > > the R9A06G032 (RZ/N1D) device. (...) > > > Using standard pinctroller bindings pin configuration properties: > > > > > > &pinctrl { > > > pinsuart0: uart0 { > > > pinsuart_tx0 { > > > pinmux = <103, UART0_I>; /* UART0_TXD */ > > > }; > > > > > > pinsuart_rx0 { > > > pinmux = <104, UART0_I>; /* UART0_RXD */ > > > bias-pull-up; > > > }; > > > }; > > > }; > > > > > > Is there anything I am missing? Maybe from the interaction with > > > "another OS/core running concurrently" you mentioned? In this case if > > > you only have to perform pin configuration (because muxing is handled > > > already) things are even simpler, just use the pin configuration > > > bindings, without involving muxing at all: > > > > > > &pinctrl { > > > pinsuart_conf: uart0 { > > > pins = <103, 104>; > > > bias-pull-up; > > > }; > > > }; > > > > Sorry I didn’t address your point. > > The only reason we want to use new properties is so the driver can process > > dts files that have been generated from an existing PinMux App. That output > > is used by VxWorks as well as our out-of-tree Linux port. If that is not a > > good enough reason to add new properties, then I can't see any technical > > reason not to use the existing bindings. > > I see. I step back then and let this to be handled by the pinctrl > subsystem people and maintainer :) I rely on Geert Uytterhoeven and Laurent Pinchart to tell me what is best for Renesas. A good hint is that Geert will merge this driver when finished and send it to me, so if he's not happy it is unlikely to get merged. So make sure Geert is happy and I will be happy too. Geert, as I, likely rely on the DT maintainers to tell me what is best for DT. But with pinctrl they often pass it back to us. Yours, Linus Walleij
On Wed, Sep 5, 2018 at 11:59 AM Phil Edworthy <phil.edworthy@renesas.com> wrote: > On 05 September 2018 10:37, Geert Uytterhoeven wrote: > > Have the VxWorks DT bindings been submitted for review to the devicetree > > mailing list? > I'm not involved with the VxWorks port, but I am pretty sure that they have > not been submitted for review. We have had other cases where deployments on other OSes have been establishing bindings (even not very pretty ones). Examples include typically the Power MacIntosh and the original SPARC bindings. Yours, Linus Walleij
On Mon, Sep 10, 2018 at 9:49 AM Linus Walleij <linus.walleij@linaro.org> wrote: > On Wed, Sep 5, 2018 at 11:59 AM Phil Edworthy <phil.edworthy@renesas.com> wrote: > > On 05 September 2018 10:37, Geert Uytterhoeven wrote: > > > > Have the VxWorks DT bindings been submitted for review to the devicetree > > > mailing list? > > > I'm not involved with the VxWorks port, but I am pretty sure that they have > > not been submitted for review. > > We have had other cases where deployments on other OSes have been > establishing bindings (even not very pretty ones). > > Examples include typically the Power MacIntosh and the original SPARC > bindings. Which predated FDT support in Linux by more than a decade, with the latter predating even p1275? While I see merit in compatibility with other existing OSes, I prefer not to deviate from standard DT bindings, if they exist. Over time, we've even migrated from Renesas-specific DT bindings to standard DT bindings, in already upstreamed SoC support. If needed, I guess a script can be written to convert DTS files from the old to the new syntax? Thanks! Gr{oetje,eeting}s, Geert
Hello, On Monday, 10 September 2018 10:48:58 EEST Linus Walleij wrote: > On Wed, Sep 5, 2018 at 11:59 AM Phil Edworthy wrote: > > On 05 September 2018 10:37, Geert Uytterhoeven wrote: > > > Have the VxWorks DT bindings been submitted for review to the devicetree > > > mailing list? > > > > I'm not involved with the VxWorks port, but I am pretty sure that they > > have not been submitted for review. > > We have had other cases where deployments on other OSes have been > establishing bindings (even not very pretty ones). I hardly see that as a good reason to accept the bindings blindly. If they haven't been submitted for review, it's their problem, not ours. Even worse in my opinion is the fact that those bindings are used by the "out-of-tree Linux port". We've hammered the "upstream first" message for years if not decades, if the industry still can't get it, I don't see why we should be the ones suffering. No ack from me for these new bindings, sorry. > Examples include typically the Power MacIntosh and the original SPARC > bindings.
Hi Geert, Laurent, On 10 September 2018 10:42 Geert Uytterhoeven wrote: > On Mon, Sep 10, 2018 at 9:49 AM Linus Walleij wrote: > > On Wed, Sep 5, 2018 at 11:59 AM Phil Edworthy wrote: > > > On 05 September 2018 10:37, Geert Uytterhoeven wrote: > > > > > > Have the VxWorks DT bindings been submitted for review to the > > > > devicetree mailing list? > > > > > I'm not involved with the VxWorks port, but I am pretty sure that > > > they have not been submitted for review. > > > > We have had other cases where deployments on other OSes have been > > establishing bindings (even not very pretty ones). > > > > Examples include typically the Power MacIntosh and the original SPARC > > bindings. > > Which predated FDT support in Linux by more than a decade, with the latter > predating even p1275? > > While I see merit in compatibility with other existing OSes, I prefer not to > deviate from standard DT bindings, if they exist. > > Over time, we've even migrated from Renesas-specific DT bindings to > standard DT bindings, in already upstreamed SoC support. > > If needed, I guess a script can be written to convert DTS files from the old to > the new syntax? Thanks for your comments, that is also my preference. Since I have to juggle effort vs. features I thought it best to send the driver with existing out-of-tree bindings first just to see. I'll modify the driver for the standard bindings. Thanks Phil
Hi Phil, On Mon, Sep 10, 2018 at 11:54 AM Phil Edworthy <phil.edworthy@renesas.com> wrote: > On 10 September 2018 10:42 Geert Uytterhoeven wrote: > > On Mon, Sep 10, 2018 at 9:49 AM Linus Walleij wrote: > > > On Wed, Sep 5, 2018 at 11:59 AM Phil Edworthy wrote: > > > > On 05 September 2018 10:37, Geert Uytterhoeven wrote: > > > > > > > > Have the VxWorks DT bindings been submitted for review to the > > > > > devicetree mailing list? > > > > > > > I'm not involved with the VxWorks port, but I am pretty sure that > > > > they have not been submitted for review. > > > > > > We have had other cases where deployments on other OSes have been > > > establishing bindings (even not very pretty ones). > > > > > > Examples include typically the Power MacIntosh and the original SPARC > > > bindings. > > > > Which predated FDT support in Linux by more than a decade, with the latter > > predating even p1275? > > > > While I see merit in compatibility with other existing OSes, I prefer not to > > deviate from standard DT bindings, if they exist. > > > > Over time, we've even migrated from Renesas-specific DT bindings to > > standard DT bindings, in already upstreamed SoC support. > > > > If needed, I guess a script can be written to convert DTS files from the old to > > the new syntax? > > Thanks for your comments, that is also my preference. Since I have to juggle > effort vs. features I thought it best to send the driver with existing out-of-tree > bindings first just to see. > > I'll modify the driver for the standard bindings. Thank you, much appreciated! Gr{oetje,eeting}s, Geert