On Thu, Feb 22, 2018 at 1:57 AM, Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> wrote:
> On 2/21/2018 6:46 AM, Andy Shevchenko wrote:
>> On Tue, Feb 20, 2018 at 11:12 PM, Thinh Nguyen
>> <Thinh.Nguyen@xxxxxxxxxxxx> wrote:
>>> On 2/17/2018 7:29 AM, Andy Shevchenko wrote:
>>>> On Fri, Feb 16, 2018 at 11:55 PM, Thinh Nguyen
>>>> <Thinh.Nguyen@xxxxxxxxxxxx> wrote:
>>>>> Add the ability to add device properties dynamically. Currently, device
>>>>> properties are added to platform device using
>>>>> platform_device_add_properties(). However, this function does not allow
>>>>> adding properties incrementally. It is useful to have this ability when
>>>>> the driver needs to set common device properties across different HW
>>>>
>>>> I'm not sure it's useful anyhow.
>>>>
>>>>> or
>>>>> if IP and FPGA validation test different configurations for different
>>>>> HW.
>>>>
>>>> Shouldn't be a separate stuff for FPGA exclusively?
>>>
>>> Can you clarify/expand your question?
>>
>> FPGA is the one which might have different properties at run time for
>> the same device.
>> So, why do we care on driver / generic level of it?
>>
>> Shouldn't be FPGA manager take care of it (via DT overlays, for example)?
>
> Normally it is handled using DT overlays. But for using FPGA as PCI
> device, I'm not aware of a better solution.
If it's a PCI device, it may utilize PCI (hot plug if you would like
to change IP at run time) with appropriate IDs and stuff, right?
>>>>> Introduce two new functions to do so:
>>>>> * dwc3_pci_add_one_property() - this function adds one property to
>>>>> dwc->properties array and increase its size as needed
>>>>> * dwc3_pci_add_properties() - this function takes a null terminated
>>>>> array of device properties and add them to dwc->properties
>>>>
>>>> So, why you can't use ACPI / DT here?
>>> dwc3_pci_add_properties() is a convenient function that takes statically
>>> allocated array of (quirks) properties for different HW and store them
>>> to dwc->properties. The idea is to add more properties on top of these
>>> required quirks.
>>
>> Yes, I understand that. What's wrong with DT? The built-in device
>> properties have the same nature as usual properties for DT.
>> Whenever driver calls device_property_read_uXX() or alike it would
>> check all property provides for asked one.
>
> I may not have explained fully in my commit message the purpose of this
> change. That's why I think I misinterpreted your previous question.
> With this new debugging feature, we want to delay adding device
> properties until the user initiates it.
So, see above.
When user wants to enable this IP at run time it will be a PCI hot
plug event which makes device appear behind PCI switch.
When device appears it would have it's own VndrID/DevID + sub pair.
Based on that IDs you may apply hard coded quirks (though I am against
quirks in new hardware) as it's done right now.
> Because the driver cannot use
> platform_device_add_properties() to incrementally add properties to
> platform device, we need a way to store properties so they can be added
> in later time.
So what? You don't need that if you do the right things right.
> That's why I added these 2 new functions.
>
>> Other than that, quirks esp. for FPGA sounds so wrong. Why in the
>> first place not to make non-broken hardware?!
>
> I may have used the term 'quirk' incorrectly since they were placed in
> dwc3_pci_quirk(). There's no quirk for FPGA, but there are some initial
> setup properties for FPGA. To be specific, these properties:
> PROPERTY_ENTRY_BOOL("snps,usb3_lpm_capable"),
> PROPERTY_ENTRY_BOOL("snps,has-lpm-erratum"),
> PROPERTY_ENTRY_BOOL("snps,dis_enblslpm_quirk"),
> PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"),
OK.
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
