Re: [PATCH 1/2] dt: bindings: Add vendor prefix for Espressif System

[Rob Herring <robh@kernel.org>]

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