Hello Mathias, On 1/6/2021 4:05 AM, Mathias Nyman wrote: > On 5.1.2021 11.30, Tejas Joglekar wrote: >> Hi Mathias, >> On 1/4/2021 9:13 PM, Greg Kroah-Hartman wrote: >>> On Mon, Jan 04, 2021 at 09:32:13AM +0000, Tejas Joglekar wrote: >>>> Hi Greg, >>>> On 1/4/2021 1:55 PM, Greg Kroah-Hartman wrote: >>>>> On Mon, Jan 04, 2021 at 01:38:43PM +0530, Tejas Joglekar wrote: >>>>>> This commit adds the platform device data to setup >>>>>> the XHCI_SG_TRB_CACHE_SIZE_QUIRK quirk. DWC3 hosts >>>>>> which are PCI devices does not use OF to create platform device >>>>>> but create xhci-plat platform device at runtime. So >>>>>> this patch allows parent device to supply the quirk >>>>>> through platform data. >>>>>> >>>>>> Signed-off-by: Tejas Joglekar <joglekar@xxxxxxxxxxxx> >>>>>> --- >>>>>> drivers/usb/dwc3/host.c | 10 ++++++++++ >>>>>> 1 file changed, 10 insertions(+) >>>>> What changed from previous versions? >>>> Resent the patch as it was missed for review by Felipe and I saw your mail >>>> >>>> to resend the patch if not reviewed. Other two patches from series are >>>> >>>> picked up by Mathias, this one is remaining for review. >>> Ah, how was I supposed to guess that? :) >>> >>>>>> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c >>>>>> index e195176580de..0434bc8cec12 100644 >>>>>> --- a/drivers/usb/dwc3/host.c >>>>>> +++ b/drivers/usb/dwc3/host.c >>>>>> @@ -11,6 +11,11 @@ >>>>>> #include <linux/platform_device.h> >>>>>> >>>>>> #include "core.h" >>>>>> +#include "../host/xhci-plat.h" >>>>> That feels really wrong. Are you sure about that? >>>> To use the struct xhci_plat_priv this was included, can you suggest alternative? >>> If that is the "normal" way to do this with the xhci driver, ok, but I >>> would like to get an ack from Mathias for this before taking it. >>> >> Can you please review this patch which is including the xhci-plat header? Let me >> >> know if anything should be modified. Ack if this patch looks ok so Greg can >> >> take it. >> > This doesn't look right. > > dwc3 shouldn't need to know about xhci platform private structures, > besides, this patch now adds the quirk to all xhci platform devices created by dwc3. As per understanding the TRB cache is present and used in all dwc3 controllers, so we would need the quirk for all. > I haven't touched dwc3 at all, but I'd guess you probably need to add a new entry to > the dwc3_pci_id_table[] in dwc3-pci.c, add a device property, and then look for that > property in xhci-plat.c, and set the quirk. The first approach was the same as you mentioned where I added device property with dt-binding for enabling the quirk, but Rob was not OK to add new bindings for each new quirk. With PCI devices compatible string would not work and dt bindings solution was not given clearance, I referred other approach taken by Peter in commit ed22764847e8100f0af9af91ccfa58e5c559bd47, (https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ed22764847e8100f0af9af91ccfa58e5c559bd47) where he used xhci platform data to pass the quirk. I followed the same approach and added the quirk. I think another way might be to make the xhci platform data structure, public so dwc3 can use it ? > -Mathias Thanks & Regards, Tejas Joglekar
