Re: [PATCH v11 05/12] mfd: zx297520v3: Add a clock and reset MFD driver

["Stefan Dösinger" <stefandoesinger@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3536 bytes --]

Hi Lee,

Thanks again for your review!

Am Donnerstag, 10. September 2026, 19:17:11 Ostafrikanische Zeit schrieb Lee 
Jones:
> On Wed, 26 Aug 2026, Stefan Dösinger wrote:

> If you use of_platform_populate() there is no issue with this living in
> soc/.
> 
> You cannot use the MFD API outside of drivers/mfd though.

I'll keep it in drivers/mfd. of_platform_populate would force me to expose the 
inner split of the CRM devices in the DT, which Conor and Krzysztof advised 
against.

> > + * Copyright (C) 2026 Stefan Dösinger
> 
> Are you sure ZTE are okay with you Copywriting a driver for their IP?

This is my free time work reverse engineering work, not something I did in 
coordination with ZTE or payment from them. I only took information about the 
hardware from their GPL dumps and not code. In my understanding the copyright 
of this code should be mine, and putting (c) ZTE would even be misleading.

ZTE provided U-Boot sources on request. Their kernel sources were already 
public before. Sadly they provided no material on their LTE components.

I didn't explicitly ask them for permission to upstream self-written drivers 
for their platform because I don't think I need it and I'd just expect a 
request like this to languish in their legal department.

The kernel once had support for a related zx296702 and zx296718 support that 
was removed in 89d4f98ae90d ("ARM: remove zte zx platform"). This code was 
written by Linaro and I believe commissioned by ZTE, so I don't think they are 
upset about me doing their work for free. At some point I'll resurrect some of 
the drivers from back then, which will then of course carry the appropriate 
Linaro and ZTE copyright notices.

> > +enum zx_device_type {
> > +	TYPE_INVALID = 0,
> > +	TYPE_TOPCRM,
> > +	TYPE_MATRIXCRM,
> > +	TYPE_LSPCRM,
> > +};
> 
> What do each of these mean?
> 
> Is the nomenclature taken from the datasheet?
> 
> If not, can we improve it to make them easier to read for humans?
> 
> Failing that, more comments would be helpful.
> 
> > +
> > +static const struct mfd_cell zx297520v3_topcrm_cells[] = {
> > +	MFD_CELL_NAME("zx297520v3-topclk"),
> > +	MFD_CELL_NAME("zx297520v3-topreset"),
> 
> Why are these works concatenated together like this?
> 
> Same with zx_device_type above?

I don't have datasheets. The names are from ZTE's mmio defines, which I expect 
to match whatever is in the datasheet.

zx297520v3 is the name ZTE gave that SoC (printed on the chip, in their kernel 
sources and boot messages). Yeah I wish it were less of a random string of 
letters and numbers. I believe it should be in the MFD cell name as a kind of 
namespacing. In the (unlikely) event hat support for a different ZX SoC is 
added some day it will bring similarly named "top"/"matrix"/"lsp" controllers

topcrm / matrixcrm / lspcrm are the names ZTE gave the 3 clock/reset/misc 
controllers. The DT binding docs in patches 1-3 elaborate on the respective 
roles of these controllers (although that understanding is based on reverse 
engineering, so it might be wrong)

Finally, "clk", "reset" are the names I gave the child nodes.

Rather than write zx297520v3-topcrm-clk, zx297520v3-matrixcrm-reset I cut out 
the "crm-" part.

Can you think of better names? Are you OK with the names if I add the above 
explanation as a comment - a little bit shortened, and making sure I don't 
repeat information that's in the bindings more than necessary?

Cheers,
Stefan

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 870 bytes --]