| Message ID | 20180327135259.30890-2-sebastian.reichel@collabora.co.uk |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | GE Healthcare PPD firmware upgrade driver for ACHC | expand |
On Tue, Mar 27, 2018 at 03:52:57PM +0200, Sebastian Reichel wrote: > This updates the GE ACHC binding, so that different compatible > strings are used for the programming interface, which is the > ezport interface from NXP MK20FN1M0VMD12 and the microcontroller's > normal SPI interface. > > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk> > --- > Documentation/devicetree/bindings/misc/ge-achc.txt | 19 ++++++++++++++++--- > 1 file changed, 16 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/misc/ge-achc.txt b/Documentation/devicetree/bindings/misc/ge-achc.txt > index 77df94d7a32f..6c6bd6568504 100644 > --- a/Documentation/devicetree/bindings/misc/ge-achc.txt > +++ b/Documentation/devicetree/bindings/misc/ge-achc.txt > @@ -7,7 +7,13 @@ Note: This device does not expose the peripherals as USB devices. > > Required properties: > > -- compatible : Should be "ge,achc" > +- compatible : Should be > + "ge,achc" (normal interface) > + "ge,achc-ezport" (flashing interface) > + > +Required properties (flashing interface only): > + > +- reset-gpios: GPIO Specifier for the reset GPIO Does the reset only affect the flashing interface and are the data pins shared? If not for both, then I think the correct thing to do here is just extend reg to support multiple values to represent multiple chip selects. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Mon, Apr 09, 2018 at 01:57:27PM -0500, Rob Herring wrote: > On Tue, Mar 27, 2018 at 03:52:57PM +0200, Sebastian Reichel wrote: > > This updates the GE ACHC binding, so that different compatible > > strings are used for the programming interface, which is the > > ezport interface from NXP MK20FN1M0VMD12 and the microcontroller's > > normal SPI interface. > > > > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk> > > --- > > Documentation/devicetree/bindings/misc/ge-achc.txt | 19 ++++++++++++++++--- > > 1 file changed, 16 insertions(+), 3 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/misc/ge-achc.txt b/Documentation/devicetree/bindings/misc/ge-achc.txt > > index 77df94d7a32f..6c6bd6568504 100644 > > --- a/Documentation/devicetree/bindings/misc/ge-achc.txt > > +++ b/Documentation/devicetree/bindings/misc/ge-achc.txt > > @@ -7,7 +7,13 @@ Note: This device does not expose the peripherals as USB devices. > > > > Required properties: > > > > -- compatible : Should be "ge,achc" > > +- compatible : Should be > > + "ge,achc" (normal interface) > > + "ge,achc-ezport" (flashing interface) > > + > > +Required properties (flashing interface only): > > + > > +- reset-gpios: GPIO Specifier for the reset GPIO > > Does the reset only affect the flashing interface and are the data pins > shared? If not for both, then I think the correct thing to do here is > just extend reg to support multiple values to represent multiple chip > selects. reset affects the whole chip and the same spi data/clock pins are being used, so extending reg should work. The flashing cannot happen with the same speed, though. I'm currently encoding this by using different "spi-max-frequency" properties. I suppose I could limit it in the driver instead. I tried to come up with an example for your suggestion. Is this what you had in mind? &spi_controller { achc@multiple { /* 0 = flashing interface, 1 = normal interface */ reg = <0>, <1>; compatible = "ge,achc"; reset-gpios = <&gpio42 23 ACTIVE_LOW>; spi-max-frequency = <42>; /* max speed for normal operation */ }; }; -- Sebastian
On Mon, Apr 9, 2018 at 4:13 PM, Sebastian Reichel <sebastian.reichel@collabora.co.uk> wrote: > Hi, > > On Mon, Apr 09, 2018 at 01:57:27PM -0500, Rob Herring wrote: >> On Tue, Mar 27, 2018 at 03:52:57PM +0200, Sebastian Reichel wrote: >> > This updates the GE ACHC binding, so that different compatible >> > strings are used for the programming interface, which is the >> > ezport interface from NXP MK20FN1M0VMD12 and the microcontroller's >> > normal SPI interface. >> > >> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk> >> > --- >> > Documentation/devicetree/bindings/misc/ge-achc.txt | 19 ++++++++++++++++--- >> > 1 file changed, 16 insertions(+), 3 deletions(-) >> > >> > diff --git a/Documentation/devicetree/bindings/misc/ge-achc.txt b/Documentation/devicetree/bindings/misc/ge-achc.txt >> > index 77df94d7a32f..6c6bd6568504 100644 >> > --- a/Documentation/devicetree/bindings/misc/ge-achc.txt >> > +++ b/Documentation/devicetree/bindings/misc/ge-achc.txt >> > @@ -7,7 +7,13 @@ Note: This device does not expose the peripherals as USB devices. >> > >> > Required properties: >> > >> > -- compatible : Should be "ge,achc" >> > +- compatible : Should be >> > + "ge,achc" (normal interface) >> > + "ge,achc-ezport" (flashing interface) >> > + >> > +Required properties (flashing interface only): >> > + >> > +- reset-gpios: GPIO Specifier for the reset GPIO >> >> Does the reset only affect the flashing interface and are the data pins >> shared? If not for both, then I think the correct thing to do here is >> just extend reg to support multiple values to represent multiple chip >> selects. > > reset affects the whole chip and the same spi data/clock pins are > being used, so extending reg should work. The flashing cannot happen > with the same speed, though. I'm currently encoding this by using > different "spi-max-frequency" properties. I suppose I could limit > it in the driver instead. I tried to come up with an example for > your suggestion. Is this what you had in mind? If the max frequency is the device max, then that should be in the driver. spi-max-frequency should really only be needed if the frequency is less than the max of either the host or device. > > &spi_controller { > achc@multiple { @0 unit addresses are the 1st address. > /* 0 = flashing interface, 1 = normal interface */ > reg = <0>, <1>; You may want to put the normal interface first as that is the primary interface and would still work assuming the OS ignored extra entries. > compatible = "ge,achc"; > reset-gpios = <&gpio42 23 ACTIVE_LOW>; > spi-max-frequency = <42>; /* max speed for normal operation */ 42 Hz? Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Fri, Apr 13, 2018 at 10:33:05AM -0500, Rob Herring wrote: > On Mon, Apr 9, 2018 at 4:13 PM, Sebastian Reichel > <sebastian.reichel@collabora.co.uk> wrote: > > Hi, > > > > On Mon, Apr 09, 2018 at 01:57:27PM -0500, Rob Herring wrote: > >> On Tue, Mar 27, 2018 at 03:52:57PM +0200, Sebastian Reichel wrote: > >> > This updates the GE ACHC binding, so that different compatible > >> > strings are used for the programming interface, which is the > >> > ezport interface from NXP MK20FN1M0VMD12 and the microcontroller's > >> > normal SPI interface. > >> > > >> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk> > >> > --- > >> > Documentation/devicetree/bindings/misc/ge-achc.txt | 19 ++++++++++++++++--- > >> > 1 file changed, 16 insertions(+), 3 deletions(-) > >> > > >> > diff --git a/Documentation/devicetree/bindings/misc/ge-achc.txt b/Documentation/devicetree/bindings/misc/ge-achc.txt > >> > index 77df94d7a32f..6c6bd6568504 100644 > >> > --- a/Documentation/devicetree/bindings/misc/ge-achc.txt > >> > +++ b/Documentation/devicetree/bindings/misc/ge-achc.txt > >> > @@ -7,7 +7,13 @@ Note: This device does not expose the peripherals as USB devices. > >> > > >> > Required properties: > >> > > >> > -- compatible : Should be "ge,achc" > >> > +- compatible : Should be > >> > + "ge,achc" (normal interface) > >> > + "ge,achc-ezport" (flashing interface) > >> > + > >> > +Required properties (flashing interface only): > >> > + > >> > +- reset-gpios: GPIO Specifier for the reset GPIO > >> > >> Does the reset only affect the flashing interface and are the data pins > >> shared? If not for both, then I think the correct thing to do here is > >> just extend reg to support multiple values to represent multiple chip > >> selects. > > > > reset affects the whole chip and the same spi data/clock pins are > > being used, so extending reg should work. The flashing cannot happen > > with the same speed, though. I'm currently encoding this by using > > different "spi-max-frequency" properties. I suppose I could limit > > it in the driver instead. I tried to come up with an example for > > your suggestion. Is this what you had in mind? > > If the max frequency is the device max, then that should be in the > driver. spi-max-frequency should really only be needed if the > frequency is less than the max of either the host or device. Right. > > &spi_controller { > > achc@multiple { > > @0 > > unit addresses are the 1st address. Ok. > > /* 0 = flashing interface, 1 = normal interface */ > > reg = <0>, <1>; > > You may want to put the normal interface first as that is the primary > interface and would still work assuming the OS ignored extra entries. > > > compatible = "ge,achc"; > > reset-gpios = <&gpio42 23 ACTIVE_LOW>; > > spi-max-frequency = <42>; /* max speed for normal operation */ > > 42 Hz? This was just a random number as example. I had a look at implementing this and the Linux SPI core does not expect any devices with multiple chip selects. I can try to implement it, but I think it makes sense to gather some feedback from Mark first (added to Cc). @Mark Patch Series: https://lwn.net/Articles/750177/ Binding Discussion: https://patchwork.kernel.org/patch/10310109/ -- Sebastian
diff --git a/Documentation/devicetree/bindings/misc/ge-achc.txt b/Documentation/devicetree/bindings/misc/ge-achc.txt index 77df94d7a32f..6c6bd6568504 100644 --- a/Documentation/devicetree/bindings/misc/ge-achc.txt +++ b/Documentation/devicetree/bindings/misc/ge-achc.txt @@ -7,7 +7,13 @@ Note: This device does not expose the peripherals as USB devices. Required properties: -- compatible : Should be "ge,achc" +- compatible : Should be + "ge,achc" (normal interface) + "ge,achc-ezport" (flashing interface) + +Required properties (flashing interface only): + +- reset-gpios: GPIO Specifier for the reset GPIO Required SPI properties: @@ -19,8 +25,15 @@ Required SPI properties: Example: -spidev0: spi@0 { - compatible = "ge,achc"; +spidev1: spi@0 { + compatible = "ge,achc-ezport"; reg = <0>; + spi-max-frequency = <300000>; + reset-gpios = <&gpio3 6 GPIO_ACTIVE_LOW>; +}; + +spidev0: spi@1 { + compatible = "ge,achc"; + reg = <1>; spi-max-frequency = <1000000>; };
This updates the GE ACHC binding, so that different compatible strings are used for the programming interface, which is the ezport interface from NXP MK20FN1M0VMD12 and the microcontroller's normal SPI interface. Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk> --- Documentation/devicetree/bindings/misc/ge-achc.txt | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)