Re: [RFC v2 4/5] DT bindings documentation for Synopsys UDC platform driver

From: Ray Jui
Date: Thu Jan 19 2017 - 17:03:16 EST




On 1/19/2017 12:17 PM, Florian Fainelli wrote:
> On 01/19/2017 12:07 PM, Scott Branden wrote:
>> Hi Florian,
>>
>> On 17-01-19 11:40 AM, Florian Fainelli wrote:
>>> On 01/19/2017 11:30 AM, Scott Branden wrote:
>>>> Hi Rob,
>>>>
>>>> On 17-01-19 09:36 AM, Rob Herring wrote:
>>>>> On Tue, Jan 17, 2017 at 01:35:07PM +0530, Raviteja Garimella wrote:
>>>>>> This patch adds device tree bindings documentation for Synopsys
>>>>>> USB device controller platform driver.
>>>>>
>>>>> Bindings describe h/w, not drivers.
>>>>>>
>>>>>> Signed-off-by: Raviteja Garimella <raviteja.garimella@xxxxxxxxxxxx>
>>>>>> ---
>>>>>> .../devicetree/bindings/usb/snps,dw-ahb-udc.txt | 27
>>>>>> ++++++++++++++++++++++
>>>>>> 1 file changed, 27 insertions(+)
>>>>>> create mode 100644
>>>>>> Documentation/devicetree/bindings/usb/snps,dw-ahb-udc.txt
>>>>>>
>>>>>> diff --git
>>>>>> a/Documentation/devicetree/bindings/usb/snps,dw-ahb-udc.txt
>>>>>> b/Documentation/devicetree/bindings/usb/snps,dw-ahb-udc.txt
>>>>>> new file mode 100644
>>>>>> index 0000000..0c18327
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/usb/snps,dw-ahb-udc.txt
>>>>>> @@ -0,0 +1,27 @@
>>>>>> +Synopsys USB Device controller.
>>>>>> +
>>>>>> +The device node is used for Synopsys Designware Cores AHB
>>>>>> +Subsystem Device Controller (UDC).
>>>>>> +
>>>>>> +This device node is used by UDCs integrated it Broadcom's
>>>>>> +Northstar2 and Cygnus SoC's.
>>>>>
>>>>> You need compatible strings for these in addition.
>>>>>
>>>> We don't need compatibility strings when an IP block is integrated into
>>>> an SoC. Otherwise each time we add the IP block to a new SoC we would
>>>> need to update ever linux driver that supports that SoC. That doesn't
>>>> make sense?
>>>
>>> You probably do need such a thing, here is how the compatible strings
>>> for IP blocks integrated into SoCs could be used:
>>>
>>> - provide a compatible strings which describes exactly the integration
>>> of this peripheral into a given SoC, e.g: brcm,udc-ns2, the reason for
>>> that is that you want to be able to capture the specific IP block
>>> integration into a specific SoC and all its quirks
>>>
>>> - if the block has its own revision scheme (and it can be relied on),
>>> provide it: brcm,udc-v1.2 and that is probably the most meaningful
>>> compatible string for a client program here
>>>
>>> - have a some kind of fallback/catchall compatible string that describes
>>> the block: brcm,udc which may also work just fine, although is not
>>> preferred
>>>
>>> Defining compatible strings is meant to avoid making (possibly
>>> incompatible) Device Tree binding changes in the future, and you always
>>> have the liberty as a client program (OS, bootloader) to match only the
>>> compatible strings you care about, from the most specific (which
>>> includes the exact SoC) to the least specific.
>>>
>>> The key thing is that, if the full set of compatible strings are present
>>> and available, you can retroactively fix your driver to be more
>>> specific, very much less so your Device Tree blob (although there is
>>> disagreement).
>>>
>> The driver stands alone from the SoC and does not need compatibility
>> strings per SoC. New SoCs will use the exact same block.
>
> Even if you take the exact same block and put it in a different SoC,
> that's still an integration work that 99% of the time goes just fine
> because the validation worked great, and the 1% of the time where you
> need to capture an integration bug, you are glad this SoC-specific
> compatible string exists such that you can work around it in the driver.

That's a very conservative estimate. Based on my experience, it's more
like 50/50, i.e., roughly half of the time we found SoC integration
specific quirks or workaround are needed.

>
> One way to solve that is to use SoC specific compatible strings because
> that presents itself as a self-contained and standardized way, or you
> can have your driver call into a piece of code that reads the SoC
> type/revision, but AFAICT this seems to be frowned upon because it
> presents some kind of layering violation.
>
>>
>> We don't add compatibility strings to any other drivers when we add the
>> same block to a new SoC.
>
> Ideally we would define new compatible strings for each new SoC we tape
> out, yet don't necessarily match them in client programs, but just
> define them as a safeguard in case something went wrong at the
> integration stage that is discovered after the fact.
>