[PATCH v2 1/7] clk/qcom: add initial clock driver for sc7280

[Simon Glass]

Hi Caleb,

On Wed, 21 Aug 2024 at 08:49, Caleb Connolly <caleb.connolly at linaro.org> wrote:
>
>
>
> On 21/08/2024 16:37, Simon Glass wrote:
> > Hi Caleb,
> >
> > On Wed, 21 Aug 2024 at 08:11, Caleb Connolly <caleb.connolly at linaro.org> wrote:
> >>
> >> Hi Simon,
> >>>> +U_BOOT_DRIVER(gcc_sc7280) = {
> >>>> +       .name           = "gcc_sc7280",
> >>>> +       .id             = UCLASS_NOP,
> >>>> +       .of_match       = gcc_sc7280_of_match,
> >>>> +       .bind           = qcom_cc_bind,
> >>>> +       .flags          = DM_FLAG_PRE_RELOC | DM_FLAG_DEFAULT_PD_CTRL_OFF,
> >>>> +};
> >>>
> >>> This should use driver model, with a UCLASS_CLK and UCLASS_RESET
> >>
> >> Please refer to qcom_cc_bind() which binds the clock, reset, and power
> >> domain drivers.
> >
> > Gosh, why? Are these devices not in the devicetree?
>
> They are, the gcc block contains clock, reset, and pd parts. On Linux
> this is not an issue because a single device can be multiple different
> classes (e..g when you register a reset you do it for a device) whereas
> U-Boot requires a device per class.
>
> e.g. see devm_reset_controller_register() in Linux, it populates a
> struct reset_controller_dev which references the struct device created
> for the node. In U-Boot you have to create a new device which _is_ the
> reset controller.

OK, I see. Rockchip has a CRU (Clock & Reset Unit) which uses syscon
to access registers. The clock driver 'owns' the node in U-Boot and it
manually binds a reset driver. It isn't great, either.

Looking at drivers/clk/qcom/clock-sdm845.c (for example), I can't
actually find "qcom,gcc-sdm845" (for example) in U-Boot, except as a
binding. Can you please point me to the node?

Re devm_reset_controller_register(), yes the U-Boot driver model is a
lot more regular, e.g. we don't really want drivers creating their own
struct udevice. We want all devices to be created automatically by the
devicetree and most memory allocations to be done automatically. This
helps to reduce code size and execution time. You probably know all
this :-)

To a significant degree, the devicetree bindings are created without
much thought to efficient operation in U-Boot. I hope that eventually
this might change.

Anyway, with the devicetree we have, I wonder how we could do this better?

Some ideas:

1. Allow DM to bind multiple devices to each devicetree node, perhaps
as a Kconfig option (to avoid time penalty for all boards), or by
requiring a new DM_FLAG_MULTI_NODE flag. The devices would then be
independent, with no common parent device

2. As 1 but have DM create a parent 'UCLASS_MULTI' device
automatically. I am thinking that we should have a new uclass for
these things, or rename the NOP uclass. This option would allow easy
access to the parent device, if needed.

3. Implement devices which are in more than one uclass. There would
still be a primary uclass, but others can be added too. This would
involve declaring a list of secondary uclasses in the U_BOOT_DRIVER()
macro. We would then have a struct dmtag_node for each secondary
uclass, containing the ID and the uclass pointer. Certain functions
would need to be updated to support this, and again it could be behind
a Kconfig.

What do you think?

Regards,
Simon