Hi Alex,
On Tue, Sep 22, 2026 at 08:33:37AM -0500, Alex Elder wrote:
> On 9/21/26 5:59 PM, Brian Masney wrote:
> > On Fri, Sep 18, 2026 at 11:52:32AM -0500, Alex Elder wrote:
> > > Define a new platform driver that manages clock and reset signals
> > > within the TC9564 SoC. There are 21 clocks, which can only be
> > > enabled and disabled, as well as 13 reset signals.
> > >
> > > Two registers manage the state of the clocks and two others manage
> > > the state of the resets. The registers are accessed via a regmap
> > > supplied by a system controller, which coordinates access to a
> > > region of memory that will be shared with another driver.
> > >
> > > Access to the memory region is provided via a BAR on a PCIe endpoint
> > > function embedded in the TC9564 SoC. For that reason, neither the
> > > PCIe clock nor PCIe reset can be manipulated by this driver (they
> > > are assumed always on and deasserted, respectively).
> > >
> > > Similarly, control is not available for the I2C clock and reset,
> > > because the PCIe subsystem on the TC9564 relies on I2C
> > >
> > > Co-developed-by: Daniel Thompson <[email protected]>
> > > Signed-off-by: Daniel Thompson <[email protected]>
> > > Signed-off-by: Alex Elder <[email protected]>
> > > ---
> > > MAINTAINERS | 1 +
> > > drivers/clk/Kconfig | 11 ++
> > > drivers/clk/Makefile | 1 +
> > > drivers/clk/clk-tc9564.c | 366 +++++++++++++++++++++++++++++++++++++++
> > > 4 files changed, 379 insertions(+)
> > > create mode 100644 drivers/clk/clk-tc9564.c
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 66d0e7e65adcb..39346a5cd9a7f 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -27675,6 +27675,7 @@ M: Alex Elder <[email protected]>
> > > M: Daniel Thompson <[email protected]>
> > > S: Maintained
> > > F:
> > > Documentation/devicetree/bindings/clock/toshiba,tc9564-clock.yaml
> > > +F: drivers/clk/clk-tc9564.c
> > > F: include/dt-bindings/clock/toshiba,tc9564.h
> > > TOSHIBA TC9564 PCI DRIVER
> > > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> > > index f9592fd9ec2bb..50efa10d48450 100644
> > > --- a/drivers/clk/Kconfig
> > > +++ b/drivers/clk/Kconfig
> > > @@ -292,6 +292,17 @@ config COMMON_CLK_S2MPS11
> > > clock. These multi-function devices have two (S2MPS14) or
> > > three
> > > (S2MPS11, S5M8767) fixed-rate oscillators, clocked at 32KHz
> > > each.
> > > +config COMMON_CLK_TC9564
> > > + tristate "Toshiba TC9564 clock support"
> > > + depends on TC9564_PCI
> >
> > select RESET_CONTROLLER
>
> Thank you. The reset and clock drivers were previously separate
> and the reset only became available if RESET_CONTROLLER was enabled.
> Combining them means I need this. I will add it.
>
> I also need to select MFD_SYSCON to get syscon_node_to_regmap()
> (or perhaps something similar, depending on the answers to my
> questions about the proper way to define the syscon).
> >
> > > + default m
> >
> > default n
>
> This depends on TC9564_PCI, and if TC9564_PCI is enabled I would
> like this to (automatically) be available (at least as a module).
>
> Why do you recommend "n"?
There is no existing 'default m' or 'default y' in the toplevel clk Kconfig.
How about this instead?
depends on TC9564_PCI || COMPILE_TEST
default TC9564_PCI
> > > + help
> > > + This enables support for the clock and reset controller embedded
> > > + in the Toshiba TC9564 (and Qualcomm QPS615) SoC. The state of
> > > + clock and reset lines is controlled by MMIO to a region managed
> > > + by a system controller; this ensures access to the region is
> > > + coordinated between this and other drivers.
> >
> > Rather than 'other drivers', outline specifically which other drivers.
>
> I was intentionally vague at this point because the one other driver
> has not yet gone out for upstream review for this iteration of the
> code. But I do agree with you, so I'll change this to say:
>
> ... between this and the XGMAC (stmmac) driver.
>
> Then it will be correct without modification once that driver goes
> out for review. Is that better, or do you suggest something else?
Yes that sounds good.
> > > +static const struct tc9564_clock_init tc9564_clock_init[] = {
> > > + TC9564_CLOCK_INIT0(MCU, 0),
> > > + TC9564_CLOCK_INIT0(INTC, 4),
> > > + /* TC9564_CLOCK_INIT0(PCIE, 9), */
> > > + /* TC9564_CLOCK_INIT0(I2C, 12), */
> >
> > A comment would be useful here to outline why these are commented out.
> > Is the PCIE one comment out because of what's outlined in the commit
> > message?
>
> Yes, that is why. We are downstream of PCIe, and PCIe in this
> case depends on I2C. So we don't want to mess with these two
> clocks or their reset counterparts.
>
> I only provide this for the benefit of documentation. If someone
> happens to see bit 9 set in this register, it means the PCIe clock
> is enabled.
>
> Anyway, others have suggested simply removing these comments.
> So I'll do what you suggest, but I'm interested to know whether
> you would favor just deleting them instead.
I also think to delete these comments. Would it make sense to include
the comment in just include/dt-bindings/clock/toshiba,tc9564.h ?
Brian