Re: [PATCH 0/6] usb: dwc3: Calculate REFCLKPER et. al. from reference clock

[Robert Hancock]

On Tue, 2022-01-18 at 20:00 +0000, Thinh Nguyen wrote:
> Sean Anderson wrote:
> > Hi Thinh,
> > 
> > On 1/18/22 2:46 PM, Thinh Nguyen wrote:
> > > Hi Sean,
> > > 
> > > Baruch Siach wrote:
> > > > Hi Sean, Thinh,
> > > > 
> > > > On Fri, Jan 14 2022, Sean Anderson wrote:
> > > > > This is a rework of patches 3-5 of [1]. It attempts to correctly
> > > > > program
> > > > > REFCLKPER and REFCLK_FLADJ based on the reference clock frequency.
> > > > > Since
> > > > > we no longer need a special property duplicating this configuration,
> > > > > snps,ref-clock-period-ns is deprecated.
> > > > > 
> > > > > Please test this! Patches 3/4 in this series have the effect of
> > > > > programming REFCLKPER and REFCLK_FLADJ on boards which already
> > > > > configure
> > > > > the "ref" clock. I have build tested, but not much else.
> > > > > 
> > > > > [1]
> > > > > https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20220114044230.2677283-1-robert.hancock@xxxxxxxxxx/__;!!A4F2R9G_pg!M3zKxDZC9a_etqzXo7GSEMTHRWfc1wR_84wwM4-fShiA35CsGcxcTEffHPbprbdC4d2R$
> > > > > 
> > > > 
> > > > Thinh, you suggested the dedicated DT property for the reference clock:
> > > > 
> > > >  
> > > > https://urldefense.com/v3/__https://lore.kernel.org/all/d5acb192-80b9-36f7-43f5-81f21c4e6ba0@xxxxxxxxxxxx/__;!!A4F2R9G_pg!M3zKxDZC9a_etqzXo7GSEMTHRWfc1wR_84wwM4-fShiA35CsGcxcTEffHPbprbpOFmvX$
> > > > 
> > > > 
> > > > Can you comment on this series?
> > > > 
> > > 
> > > Unless there's a good way to pass this information for PCI devices, my
> > > opinion hasn't changed. (Btw, I don't think creating a dummy clock
> > > provider and its dummy ops is a good solution as seems to complicate and
> > > bloat the PCI glue drivers).
> > 
> > Can you explain your situation a bit more? I'm not sure how you can
> > access a device tree property but not add a fixed-rate clock.
> > 
> > --Sean
> 
> Currently for dwc3 pci devices, we have glue drivers that create a
> platform_device with specific properties to pass to the dwc3 core
> driver. Without a ref clock property, we would need another way to pass
> this information to the core driver or another way for the dwc3 core
> driver to check for specific pci device's properties and quirks.

We've used the device tree to instantiate/configure devices inside of a PCI
device, though obviously that only works on DT-based platforms, and for
hardware that's part of the board itself, not an add-in card.

We've also used the MFD infrastructure to instantiate devices and device
properties inside a PCI device on x86, which can be used if the driver you are
instantiating uses the generic device property accessors and not the DT-
specific ones. That gets a bit dirty however - I don't think there's an easy
way to create properties that are references to other nodes, or more than a
single level deep heirarchy of nodes.

For a use case like you're describing, it sounds like it would be better to
abstract away some of the core DWC3 code from reading the settings from DT
directly, so that the PCI devices can instantiate it and set the configuration
however they want, without having to worry about creating fake properties for
the core to read. I think that pattern has been used with some other drivers
such as AHCI?

> 
> BR,
> Thinh
-- 
Robert Hancock
Senior Hardware Designer, Calian Advanced Technologies
www.calian.com