Re: [PATCH v3 0/9] Add the Renesas USBF controller support

[Herve Codina]

Hi Rob,

On Wed, 7 Dec 2022 16:19:42 -0600
Rob Herring <robh+dt@xxxxxxxxxx> wrote:

> On Wed, Dec 7, 2022 at 10:24 AM Herve Codina <herve.codina@xxxxxxxxxxx> wrote:
> >
> > Hi,
> >
> > This series add support for the Renesas USBF controller (USB Device
> > Controller) available in the Renesas RZ/N1 SoC.
> >
> > Based on previous review:
> >   https://lore.kernel.org/all/20221114111513.1436165-3-herve.codina@xxxxxxxxxxx/
> >
> > A new strategy is proposed to handle the H2MODE bit from CFG_USB
> > register compared to the previous versions on the series. As a
> > reminder, H2MODE bit allows to configure the internal USB Port
> > interface for two hosts or one host and one device.  
> 
> Is this case any different from all the phandle properties we have in
> bindings that point to some misc registers somewhere else you need to
> poke? If so, I'm not really a fan of duplicating the information.

Our case is that there is a bit in a register that affect several
devices. This bit must be set before the devices are started.
If this bit is changed while affected devices are running, system
hangs can occurs (datasheet).

So, in order to do that we need the device in charge to set
this bit (sysctrl) to set this bit before other devices (USBF
and PCI bridge) were started.

At sysctrl level, the bit is set during the probe() call.
The property 'depends-on' aim is to ensure the probe() calls
order between provider (sysctrl) and consumers (USBF and PCI
bridge).

regmap and syscon are used to export registers from one device
to an other and the probe() calls order is not ensured by the
core or regmap infrastructure. Indeed, the regmap provider
probe() will not be called if the regmap provider was not probed
before the consumer ask for the regmap.
  https://elixir.bootlin.com/linux/latest/source/drivers/mfd/syscon.c#L152
  https://elixir.bootlin.com/linux/latest/source/drivers/mfd/syscon.c#L43
No specific action synchronisation are done with regmap/syscon
other than the regmap creation itself.

I don't think the regmap/syscon will help in our case.

> 
> We also have cases of of_find_compatible_node(NULL, NULL,
> "foo-bar-syscon") which is a dependency expressed in the driver, but
> not DT. In either case, adding 'depends-on' would be an ABI break as
> you are requiring a DT change.

In order to avoid the DT change, I can keep the 'depends-on'
optional in the PCI bridge binding.
This will be functionnal as sysctrl is already used in this node
(power-domain = <&sysctrl>). The relationship is already present
with this power-domain link.

If ok, I will do this change in v4 series.

> 
> > This new strategy is:
> >   - Add the new generic 'depends-on' property in the device tree.
> >
> >     This generic property expresses an simple functionnal dependency
> >     that does not rely on a specific topic. It is an 'order only'
> >     dependency that can be used for dependencies between consumers
> >     and producers that are not based on a specific infrastructure
> >     and not need other relationship than this simple 'order only'
> >     (ie no API is provided between the provider and the consumer)  
> 
> Isn't getting a regmap an API?
> 
> Rob

-- 
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com