Re: [PATCHv2 1/3] dt-bindings: misc: achc: Make ezport distinguishable
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 > 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 > >> > --- > >> > 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 signature.asc Description: PGP signature
Re: [PATCHv2 1/3] dt-bindings: misc: achc: Make ezport distinguishable
On Mon, Apr 9, 2018 at 4:13 PM, Sebastian Reichel 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 >> > --- >> > 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
Re: [PATCHv2 1/3] dt-bindings: misc: achc: Make ezport distinguishable
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 > > --- > > 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 signature.asc Description: PGP signature
Re: [PATCHv2 1/3] dt-bindings: misc: achc: Make ezport distinguishable
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 > --- > 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