Hi, John Youn <John.Youn@xxxxxxxxxxxx> writes: > On 02/22/2018 07:20 AM, Andy Shevchenko wrote: >> 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. >> > > Hi Andy, > > Using VID/PID is not really feasible for our use case. If we only had > a few concrete devices then it would be ok. Agreed. VID/PID would not scale at all :-) > But understand that this an IP running on an FPGA validation > platform. The IP is very large and flexible with *many* parameters > that we must test against. It is also deployed by our customers with > widely varying configurations. So we are constantly testing these as > well. > > One of the last pieces for moving our FPGA validation completely to > the in-kernel Linux stack is the ability to configure the driver to > set parameters that are not visible to the driver, or with parameters > that we want to constrain for testing. I'm very much interested in helping you guys validate your IP with upstream Linux :-) My concern with this patch is that it makes dwc3-pci super complex even for users who are not interested in IP validation. So, instead, how about we introduce dwc3-snps.c where you guys can put all the controls you need? We remove HAPS from current dwc3-pci.c and move everything to dwc3-snps.c. Sure, we'd have a little duplication, but I guess that'd be very minor. While doing that, we can make dwc3-snps.c depend on EXPERT, so that distros don't enable it by default. I don't mind adding a bunch of properties to dwc3 core as long as it has an actual use. I guess maintaining that is far from being a problem, however I don't want to have impacts on actual products since this is only necessary for validation activities :-) cheers -- balbi
Attachment:
signature.asc
Description: PGP signature
