| Message ID | 1470596269-20572-1-git-send-email-hdegoede@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Sunday, August 7, 2016 8:57:49 PM CEST Hans de Goede wrote: > +Espressif ESP8089 wireless SDIO devices > + > +This node provides properties for controlling the ESP8089 wireless device. > +The node is expected to be specified as a child node to the SDIO controller > +that connects the device to the system. > + > +Required properties: > + > + - compatible : Should be "esp,esp8089". I think it would be good to standardize a compatible string for sdio like we have it for other discoverable buses, based on the vendor/device ID (or whatever this uses), using something like compatible = "sdio1234,5678", "esp,esp8089"; > +Optional properties: > + - esp,crystal_26M_en: Integer value for the crystal_26M_en firmware parameter better use '-' instead of '_' for property names. Arnd
08.08.2016, 02:58, "Hans de Goede" <hdegoede@redhat.com>: > The ESP8089 chips can mostly be enumerated via their sdio interface, > but they are clocked by an external crystal which may differ from one > board to the other. > > This commit adds a binding for the sdio child node for these chips, > allowing to specify the external crystal type (for now, this binding > could be be extended with e.g. OOB irq support later). > > The Android driver for this chip uses a text file with key,value pairs > which gets loaded as firmware to pass this info to the firmware. > The "esp,crystal_26M_en" name is chosen to match the crystal_26M_en > key-name in that text file. > > Note that at this point there only is an out of tree driver for this > hardware, there is no clear timeline / path for merging this. Still > I believe it would be good to specify the binding for this in tree > now, so that any future migration to an in tree driver will not cause > compatiblity issues. > > Cc: Icenowy Zheng <icenowy@aosc.xyz> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > .../bindings/net/wireless/esp,esp8089.txt | 31 ++++++++++++++++++++++ > 1 file changed, 31 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/wireless/esp,esp8089.txt > > diff --git a/Documentation/devicetree/bindings/net/wireless/esp,esp8089.txt b/Documentation/devicetree/bindings/net/wireless/esp,esp8089.txt > new file mode 100644 > index 0000000..898a149 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/wireless/esp,esp8089.txt > @@ -0,0 +1,31 @@ > +Espressif ESP8089 wireless SDIO devices > + > +This node provides properties for controlling the ESP8089 wireless device. > +The node is expected to be specified as a child node to the SDIO controller > +that connects the device to the system. > + > +Required properties: > + > + - compatible : Should be "esp,esp8089". There's also esp8266. (And the difference of 8089 and 8266 is mainly the difference of crystal frequency... 26MHz for 8266, 40MHz for 8089) So maybe it should be the difference of dt compatible? > + > +Optional properties: > + - esp,crystal_26M_en: Integer value for the crystal_26M_en firmware parameter > + > +Example: > + > +&mmc1 { > + #address-cells = <1>; > + #size-cells = <0>; > + > + vmmc-supply = <®_dldo1>; > + mmc-pwrseq = <&wifi_pwrseq>; > + bus-width = <4>; > + non-removable; > + status = "okay"; > + > + esp8089: sdio_wifi@1 { > + compatible = "esp,esp8089"; > + reg = <1>; > + esp,crystal_26M_en = <2>; > + }; > +}; > -- > 2.7.4
Hi Arnd, On Mon, Aug 8, 2016 at 3:59 AM, Arnd Bergmann <arnd@arndb.de> wrote: > On Sunday, August 7, 2016 8:57:49 PM CEST Hans de Goede wrote: >> +Espressif ESP8089 wireless SDIO devices >> + >> +This node provides properties for controlling the ESP8089 wireless device. >> +The node is expected to be specified as a child node to the SDIO controller >> +that connects the device to the system. >> + >> +Required properties: >> + >> + - compatible : Should be "esp,esp8089". > > I think it would be good to standardize a compatible string for sdio > like we have it for other discoverable buses, based on the > vendor/device ID (or whatever this uses), using something like > > compatible = "sdio1234,5678", "esp,esp8089"; Is there a central repository for SDIO vendor/device IDs similar to PCI or USB IDs? The vendor ID Espressif uses is 0x6666, which seems kind of bogus to me. ChenYu > >> +Optional properties: >> + - esp,crystal_26M_en: Integer value for the crystal_26M_en firmware parameter > > better use '-' instead of '_' for property names. > > Arnd > >
Hi, On 08-08-16 04:36, Icenowy Zheng wrote: > > > 08.08.2016, 02:58, "Hans de Goede" <hdegoede@redhat.com>: >> The ESP8089 chips can mostly be enumerated via their sdio interface, >> but they are clocked by an external crystal which may differ from one >> board to the other. >> >> This commit adds a binding for the sdio child node for these chips, >> allowing to specify the external crystal type (for now, this binding >> could be be extended with e.g. OOB irq support later). >> >> The Android driver for this chip uses a text file with key,value pairs >> which gets loaded as firmware to pass this info to the firmware. >> The "esp,crystal_26M_en" name is chosen to match the crystal_26M_en >> key-name in that text file. >> >> Note that at this point there only is an out of tree driver for this >> hardware, there is no clear timeline / path for merging this. Still >> I believe it would be good to specify the binding for this in tree >> now, so that any future migration to an in tree driver will not cause >> compatiblity issues. >> >> Cc: Icenowy Zheng <icenowy@aosc.xyz> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> .../bindings/net/wireless/esp,esp8089.txt | 31 ++++++++++++++++++++++ >> 1 file changed, 31 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/net/wireless/esp,esp8089.txt >> >> diff --git a/Documentation/devicetree/bindings/net/wireless/esp,esp8089.txt b/Documentation/devicetree/bindings/net/wireless/esp,esp8089.txt >> new file mode 100644 >> index 0000000..898a149 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/net/wireless/esp,esp8089.txt >> @@ -0,0 +1,31 @@ >> +Espressif ESP8089 wireless SDIO devices >> + >> +This node provides properties for controlling the ESP8089 wireless device. >> +The node is expected to be specified as a child node to the SDIO controller >> +that connects the device to the system. >> + >> +Required properties: >> + >> + - compatible : Should be "esp,esp8089". > There's also esp8266. (And the difference of 8089 and 8266 is mainly the difference of crystal frequency... 26MHz for 8266, 40MHz for 8089) > So maybe it should be the difference of dt compatible? 2 of my tablets (both Polaroid ones) use a value of 2 (rather then 0 or 1) for crystal_26M_en and using a different value does not work. Another generic q8 form-factor tablet I've with an a23 needs crystal_26M_en=1. In all these tablets the chip is clearly marked esp8089, so the crystal_26M_en setting seems to not be related to the chip model and deriving this from the compatible string seems like a bad idea to me. Regards, Hans >> + >> +Optional properties: >> + - esp,crystal_26M_en: Integer value for the crystal_26M_en firmware parameter >> + >> +Example: >> + >> +&mmc1 { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + vmmc-supply = <®_dldo1>; >> + mmc-pwrseq = <&wifi_pwrseq>; >> + bus-width = <4>; >> + non-removable; >> + status = "okay"; >> + >> + esp8089: sdio_wifi@1 { >> + compatible = "esp,esp8089"; >> + reg = <1>; >> + esp,crystal_26M_en = <2>; >> + }; >> +}; >> -- >> 2.7.4
On Monday, August 8, 2016 11:59:25 AM CEST Chen-Yu Tsai wrote: > Hi Arnd, > > On Mon, Aug 8, 2016 at 3:59 AM, Arnd Bergmann <arnd@arndb.de> wrote: > > On Sunday, August 7, 2016 8:57:49 PM CEST Hans de Goede wrote: > >> +Espressif ESP8089 wireless SDIO devices > >> + > >> +This node provides properties for controlling the ESP8089 wireless device. > >> +The node is expected to be specified as a child node to the SDIO controller > >> +that connects the device to the system. > >> + > >> +Required properties: > >> + > >> + - compatible : Should be "esp,esp8089". > > > > I think it would be good to standardize a compatible string for sdio > > like we have it for other discoverable buses, based on the > > vendor/device ID (or whatever this uses), using something like > > > > compatible = "sdio1234,5678", "esp,esp8089"; > > Is there a central repository for SDIO vendor/device IDs similar to > PCI or USB IDs? No idea. I believe the existing repositories for PCI and USB are not public either, there is just the information at http://pcidatabase.com/, http://pci-ids.ucw.cz/ or http://www.linux-usb.org/usb.ids that are maintained by volunteers and that are not authoritarive. I found one list at https://github.com/systemd/systemd/blob/master/hwdb/sdio.ids but it's probably very incomplete. > The vendor ID Espressif uses is 0x6666, which seems kind of bogus to me. It probably is, but that's not important as long as nothing else uses that ID. Arnd
Hi, 08.08.2016, 15:34, "Hans de Goede" <hdegoede@redhat.com>: > Hi, > > On 08-08-16 04:36, Icenowy Zheng wrote: >> 08.08.2016, 02:58, "Hans de Goede" <hdegoede@redhat.com>: >>> The ESP8089 chips can mostly be enumerated via their sdio interface, >>> but they are clocked by an external crystal which may differ from one >>> board to the other. >>> >>> This commit adds a binding for the sdio child node for these chips, >>> allowing to specify the external crystal type (for now, this binding >>> could be be extended with e.g. OOB irq support later). >>> >>> The Android driver for this chip uses a text file with key,value pairs >>> which gets loaded as firmware to pass this info to the firmware. >>> The "esp,crystal_26M_en" name is chosen to match the crystal_26M_en >>> key-name in that text file. >>> >>> Note that at this point there only is an out of tree driver for this >>> hardware, there is no clear timeline / path for merging this. Still >>> I believe it would be good to specify the binding for this in tree >>> now, so that any future migration to an in tree driver will not cause >>> compatiblity issues. >>> >>> Cc: Icenowy Zheng <icenowy@aosc.xyz> >>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>> --- >>> .../bindings/net/wireless/esp,esp8089.txt | 31 ++++++++++++++++++++++ >>> 1 file changed, 31 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/net/wireless/esp,esp8089.txt >>> >>> diff --git a/Documentation/devicetree/bindings/net/wireless/esp,esp8089.txt b/Documentation/devicetree/bindings/net/wireless/esp,esp8089.txt >>> new file mode 100644 >>> index 0000000..898a149 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/net/wireless/esp,esp8089.txt >>> @@ -0,0 +1,31 @@ >>> +Espressif ESP8089 wireless SDIO devices >>> + >>> +This node provides properties for controlling the ESP8089 wireless device. >>> +The node is expected to be specified as a child node to the SDIO controller >>> +that connects the device to the system. >>> + >>> +Required properties: >>> + >>> + - compatible : Should be "esp,esp8089". >> There's also esp8266. (And the difference of 8089 and 8266 is mainly the difference of crystal frequency... 26MHz for 8266, 40MHz for 8089) >> So maybe it should be the difference of dt compatible? > > 2 of my tablets (both Polaroid ones) use a value of 2 (rather > then 0 or 1) for crystal_26M_en and using a different value > does not work. Another generic q8 form-factor tablet I've with > an a23 needs crystal_26M_en=1. In all these tablets the chip is > clearly marked esp8089, so the crystal_26M_en setting seems to > not be related to the chip model and deriving this from the > compatible string seems like a bad idea to me. Oh such strange. Thanks, Icenowy > > Regards, > > Hans > >>> + >>> +Optional properties: >>> + - esp,crystal_26M_en: Integer value for the crystal_26M_en firmware parameter >>> + >>> +Example: >>> + >>> +&mmc1 { >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + vmmc-supply = <®_dldo1>; >>> + mmc-pwrseq = <&wifi_pwrseq>; >>> + bus-width = <4>; >>> + non-removable; >>> + status = "okay"; >>> + >>> + esp8089: sdio_wifi@1 { >>> + compatible = "esp,esp8089"; >>> + reg = <1>; >>> + esp,crystal_26M_en = <2>; >>> + }; >>> +}; >>> -- >>> 2.7.4
Hi, 08.08.2016, 17:30, "Arnd Bergmann" <arnd@arndb.de>: > On Monday, August 8, 2016 11:59:25 AM CEST Chen-Yu Tsai wrote: >> Hi Arnd, >> >> On Mon, Aug 8, 2016 at 3:59 AM, Arnd Bergmann <arnd@arndb.de> wrote: >> > On Sunday, August 7, 2016 8:57:49 PM CEST Hans de Goede wrote: >> >> +Espressif ESP8089 wireless SDIO devices >> >> + >> >> +This node provides properties for controlling the ESP8089 wireless device. >> >> +The node is expected to be specified as a child node to the SDIO controller >> >> +that connects the device to the system. >> >> + >> >> +Required properties: >> >> + >> >> + - compatible : Should be "esp,esp8089". >> > >> > I think it would be good to standardize a compatible string for sdio >> > like we have it for other discoverable buses, based on the >> > vendor/device ID (or whatever this uses), using something like >> > >> > compatible = "sdio1234,5678", "esp,esp8089"; >> >> Is there a central repository for SDIO vendor/device IDs similar to >> PCI or USB IDs? > > No idea. I believe the existing repositories for PCI and USB are not > public either, there is just the information at http://pcidatabase.com/, > http://pci-ids.ucw.cz/ or http://www.linux-usb.org/usb.ids that > are maintained by volunteers and that are not authoritarive. > > I found one list at https://github.com/systemd/systemd/blob/master/hwdb/sdio.ids > but it's probably very incomplete. Maybe it should be maintained as a independent project... > >> The vendor ID Espressif uses is 0x6666, which seems kind of bogus to me. > > It probably is, but that's not important as long as nothing else uses > that ID. Yes... I think the USB vendor ID of Allwinner, 0x1f3a, is also nothing authoritative. > > Arnd Thanks, Icenowy
On Sun, Aug 07, 2016 at 08:57:48PM +0200, Hans de Goede wrote: > Espressif is a manufacturer of various wifi and bt chips, add a vendor > prefix for use with bindings for these chips. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > Documentation/devicetree/bindings/vendor-prefixes.txt | 1 + > 1 file changed, 1 insertion(+) Acked-by: Rob Herring <robh@kernel.org>
Hi Rob, On 10-08-16 20:48, Rob Herring wrote: > On Sun, Aug 07, 2016 at 08:57:48PM +0200, Hans de Goede wrote: >> Espressif is a manufacturer of various wifi and bt chips, add a vendor >> prefix for use with bindings for these chips. >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> Documentation/devicetree/bindings/vendor-prefixes.txt | 1 + >> 1 file changed, 1 insertion(+) > > Acked-by: Rob Herring <robh@kernel.org> Thank you for all the reviews. What about: "[PATCH 2/2] dt: bindings: Add binding for ESP8089 wifi chips" ? there were some remarks, but I believe that those have been addressed (by answering the remarks, not with a new version), so a review of that one would be appreciated too. Also who should merge this series, Ulf because it is mmc related, or the dt-maintainers since it contains only bindings patches ? Regards, Hans
On Thu, Aug 11, 2016 at 4:15 AM, Hans de Goede <hdegoede@redhat.com> wrote: > Hi Rob, > > > On 10-08-16 20:48, Rob Herring wrote: >> >> On Sun, Aug 07, 2016 at 08:57:48PM +0200, Hans de Goede wrote: >>> >>> Espressif is a manufacturer of various wifi and bt chips, add a vendor >>> prefix for use with bindings for these chips. >>> >>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>> --- >>> Documentation/devicetree/bindings/vendor-prefixes.txt | 1 + >>> 1 file changed, 1 insertion(+) >> >> >> Acked-by: Rob Herring <robh@kernel.org> > > > Thank you for all the reviews. > > What about: "[PATCH 2/2] dt: bindings: Add binding for ESP8089 wifi chips" ? > there were some remarks, but I believe that those have been addressed > (by answering the remarks, not with a new version), so a review of that > one would be appreciated too. What about Arnd's comment "better use '-' instead of '_' for property names." > Also who should merge this series, Ulf because it is mmc related, > or the dt-maintainers since it contains only bindings patches ? I can take it. Rob
Hi, On 11-08-16 15:31, Rob Herring wrote: > On Thu, Aug 11, 2016 at 4:15 AM, Hans de Goede <hdegoede@redhat.com> wrote: >> Hi Rob, >> >> >> On 10-08-16 20:48, Rob Herring wrote: >>> >>> On Sun, Aug 07, 2016 at 08:57:48PM +0200, Hans de Goede wrote: >>>> >>>> Espressif is a manufacturer of various wifi and bt chips, add a vendor >>>> prefix for use with bindings for these chips. >>>> >>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>>> --- >>>> Documentation/devicetree/bindings/vendor-prefixes.txt | 1 + >>>> 1 file changed, 1 insertion(+) >>> >>> >>> Acked-by: Rob Herring <robh@kernel.org> >> >> >> Thank you for all the reviews. >> >> What about: "[PATCH 2/2] dt: bindings: Add binding for ESP8089 wifi chips" ? >> there were some remarks, but I believe that those have been addressed >> (by answering the remarks, not with a new version), so a review of that >> one would be appreciated too. > > What about Arnd's comment "better use '-' instead of '_' for property names." Ah yes, I forgot about that comment. As explained in the commit msg I named the dt property to be the same as the keyword used in the ini-like config file android uses for this (the android driver loads a file with board specific esp8089 config like the crystal-type from /lib/firmware). And that file does use '_'. I want to keep the names the same as that seems the sensible thing to do, but if there is a great preference for '-' in dt I can do a v2 with that changed. >> Also who should merge this series, Ulf because it is mmc related, >> or the dt-maintainers since it contains only bindings patches ? > > I can take it. Great, thanks. Regards, Hans
On Thu, Aug 11, 2016 at 10:08 AM, Hans de Goede <hdegoede@redhat.com> wrote: > Hi, > > On 11-08-16 15:31, Rob Herring wrote: >> >> On Thu, Aug 11, 2016 at 4:15 AM, Hans de Goede <hdegoede@redhat.com> >> wrote: >>> >>> Hi Rob, >>> >>> >>> On 10-08-16 20:48, Rob Herring wrote: >>>> >>>> >>>> On Sun, Aug 07, 2016 at 08:57:48PM +0200, Hans de Goede wrote: >>>>> >>>>> >>>>> Espressif is a manufacturer of various wifi and bt chips, add a vendor >>>>> prefix for use with bindings for these chips. >>>>> >>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>>>> --- >>>>> Documentation/devicetree/bindings/vendor-prefixes.txt | 1 + >>>>> 1 file changed, 1 insertion(+) >>>> >>>> >>>> >>>> Acked-by: Rob Herring <robh@kernel.org> >>> >>> >>> >>> Thank you for all the reviews. >>> >>> What about: "[PATCH 2/2] dt: bindings: Add binding for ESP8089 wifi >>> chips" ? >>> there were some remarks, but I believe that those have been addressed >>> (by answering the remarks, not with a new version), so a review of that >>> one would be appreciated too. >> >> >> What about Arnd's comment "better use '-' instead of '_' for property >> names." > > > Ah yes, I forgot about that comment. As explained in the commit msg > I named the dt property to be the same as the keyword used in the > ini-like config file android uses for this (the android driver loads a > file with board specific esp8089 config like the crystal-type from > /lib/firmware). > > And that file does use '_'. I want to keep the names the same as that > seems the sensible thing to do, but if there is a great preference for > '-' in dt I can do a v2 with that changed. I could see wanting to use the same string in the driver for a small size savings, but I'd assume a mainline driver would do away with the firmware text file and only use DT? Also, do you expect lots more properties? Maintaining the exact string would be more worthwhile if there are lots of properties. I plan to make '_' cause warnings with dtc which is why we don't want to add more. Rob
Hi, On 11-08-16 18:35, Rob Herring wrote: > On Thu, Aug 11, 2016 at 10:08 AM, Hans de Goede <hdegoede@redhat.com> wrote: >> Hi, >> >> On 11-08-16 15:31, Rob Herring wrote: >>> >>> On Thu, Aug 11, 2016 at 4:15 AM, Hans de Goede <hdegoede@redhat.com> >>> wrote: >>>> >>>> Hi Rob, >>>> >>>> >>>> On 10-08-16 20:48, Rob Herring wrote: >>>>> >>>>> >>>>> On Sun, Aug 07, 2016 at 08:57:48PM +0200, Hans de Goede wrote: >>>>>> >>>>>> >>>>>> Espressif is a manufacturer of various wifi and bt chips, add a vendor >>>>>> prefix for use with bindings for these chips. >>>>>> >>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>>>>> --- >>>>>> Documentation/devicetree/bindings/vendor-prefixes.txt | 1 + >>>>>> 1 file changed, 1 insertion(+) >>>>> >>>>> >>>>> >>>>> Acked-by: Rob Herring <robh@kernel.org> >>>> >>>> >>>> >>>> Thank you for all the reviews. >>>> >>>> What about: "[PATCH 2/2] dt: bindings: Add binding for ESP8089 wifi >>>> chips" ? >>>> there were some remarks, but I believe that those have been addressed >>>> (by answering the remarks, not with a new version), so a review of that >>>> one would be appreciated too. >>> >>> >>> What about Arnd's comment "better use '-' instead of '_' for property >>> names." >> >> >> Ah yes, I forgot about that comment. As explained in the commit msg >> I named the dt property to be the same as the keyword used in the >> ini-like config file android uses for this (the android driver loads a >> file with board specific esp8089 config like the crystal-type from >> /lib/firmware). >> >> And that file does use '_'. I want to keep the names the same as that >> seems the sensible thing to do, but if there is a great preference for >> '-' in dt I can do a v2 with that changed. > > I could see wanting to use the same string in the driver for a small > size savings, but I'd assume a mainline driver would do away with the > firmware text file and only use DT? Yes. > Also, do you expect lots more > properties? No, it seems that most settings from the text-file android use always are at a fixed value. > Maintaining the exact string would be more worthwhile if > there are lots of properties. > > I plan to make '_' cause warnings with dtc which is why we don't want > to add more. Ah, ok that is a good reason to change to '-' I'll do a v2 with that change. Regards, Hans
Hi, On 11-08-16 18:35, Rob Herring wrote: <snip> > I plan to make '_' cause warnings with dtc which is why we don't want > to add more. Offtopic question: does this only apply to property names, or also to node names ? Regards, Hans
On Thu, Aug 11, 2016 at 12:28 PM, Hans de Goede <hdegoede@redhat.com> wrote: > Hi, > > On 11-08-16 18:35, Rob Herring wrote: > > <snip> > >> I plan to make '_' cause warnings with dtc which is why we don't want >> to add more. > > > Offtopic question: does this only apply to property names, or > also to node names ? Both. Rob
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt index d2bce22..d83a2da 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.txt +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt @@ -88,6 +88,7 @@ energymicro Silicon Laboratories (formerly Energy Micro AS) epcos EPCOS AG epfl Ecole Polytechnique Fédérale de Lausanne epson Seiko Epson Corp. +esp Espressif System est ESTeem Wireless Modems ettus NI Ettus Research eukrea Eukréa Electromatique
Espressif is a manufacturer of various wifi and bt chips, add a vendor prefix for use with bindings for these chips. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- Documentation/devicetree/bindings/vendor-prefixes.txt | 1 + 1 file changed, 1 insertion(+)