[v6] Add Microchip ZL3073x support (part 1)

[PATCH net-next v6 8/8] mfd: zl3073x: Register DPLL sub-device during init

Posted by Ivan Vecera 9 months, 2 weeks ago

Register DPLL sub-devices to expose the functionality provided
by ZL3073x chip family. Each sub-device represents one of
the available DPLL channels.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
v4->v6:
* no change
v3->v4:
* use static mfd cells
---
 drivers/mfd/zl3073x-core.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/mfd/zl3073x-core.c b/drivers/mfd/zl3073x-core.c
index 050dc57c90c3..3e665cdf228f 100644
--- a/drivers/mfd/zl3073x-core.c
+++ b/drivers/mfd/zl3073x-core.c
@@ -7,6 +7,7 @@
 #include <linux/device.h>
 #include <linux/export.h>
 #include <linux/math64.h>
+#include <linux/mfd/core.h>
 #include <linux/mfd/zl3073x.h>
 #include <linux/module.h>
 #include <linux/netlink.h>
@@ -755,6 +756,14 @@ static void zl3073x_devlink_unregister(void *ptr)
 	devlink_unregister(ptr);
 }
 
+static const struct mfd_cell zl3073x_dpll_cells[] = {
+	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 0),
+	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 1),
+	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 2),
+	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 3),
+	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 4),
+};
+
 /**
  * zl3073x_dev_probe - initialize zl3073x device
  * @zldev: pointer to zl3073x device
@@ -826,6 +835,16 @@ int zl3073x_dev_probe(struct zl3073x_dev *zldev,
 	if (rc)
 		return rc;
 
+	/* Add DPLL sub-device cell for each DPLL channel */
+	rc = devm_mfd_add_devices(zldev->dev, PLATFORM_DEVID_AUTO,
+				  zl3073x_dpll_cells, chip_info->num_channels,
+				  NULL, 0, NULL);
+	if (rc) {
+		dev_err_probe(zldev->dev, rc,
+			      "Failed to add DPLL sub-device\n");
+		return rc;
+	}
+
 	/* Register the device as devlink device */
 	devlink = priv_to_devlink(zldev);
 	devlink_register(devlink);
-- 
2.49.0

Re: [PATCH net-next v6 8/8] mfd: zl3073x: Register DPLL sub-device during init

Posted by Lee Jones 9 months, 1 week ago

On Wed, 30 Apr 2025, Ivan Vecera wrote:

> Register DPLL sub-devices to expose the functionality provided
> by ZL3073x chip family. Each sub-device represents one of
> the available DPLL channels.
> 
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
> v4->v6:
> * no change
> v3->v4:
> * use static mfd cells
> ---
>  drivers/mfd/zl3073x-core.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/mfd/zl3073x-core.c b/drivers/mfd/zl3073x-core.c
> index 050dc57c90c3..3e665cdf228f 100644
> --- a/drivers/mfd/zl3073x-core.c
> +++ b/drivers/mfd/zl3073x-core.c
> @@ -7,6 +7,7 @@
>  #include <linux/device.h>
>  #include <linux/export.h>
>  #include <linux/math64.h>
> +#include <linux/mfd/core.h>
>  #include <linux/mfd/zl3073x.h>
>  #include <linux/module.h>
>  #include <linux/netlink.h>
> @@ -755,6 +756,14 @@ static void zl3073x_devlink_unregister(void *ptr)
>  	devlink_unregister(ptr);
>  }
>  
> +static const struct mfd_cell zl3073x_dpll_cells[] = {
> +	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 0),
> +	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 1),
> +	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 2),
> +	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 3),
> +	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 4),
> +};

What other devices / subsystems will be involved when this is finished?

-- 
Lee Jones [李琼斯]

Re: [PATCH net-next v6 8/8] mfd: zl3073x: Register DPLL sub-device during init

Posted by Ivan Vecera 9 months, 1 week ago


On 01. 05. 25 3:22 odp., Lee Jones wrote:
> On Wed, 30 Apr 2025, Ivan Vecera wrote:
> 
>> Register DPLL sub-devices to expose the functionality provided
>> by ZL3073x chip family. Each sub-device represents one of
>> the available DPLL channels.
>>
>> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
>> ---
>> v4->v6:
>> * no change
>> v3->v4:
>> * use static mfd cells
>> ---
>>   drivers/mfd/zl3073x-core.c | 19 +++++++++++++++++++
>>   1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/mfd/zl3073x-core.c b/drivers/mfd/zl3073x-core.c
>> index 050dc57c90c3..3e665cdf228f 100644
>> --- a/drivers/mfd/zl3073x-core.c
>> +++ b/drivers/mfd/zl3073x-core.c
>> @@ -7,6 +7,7 @@
>>   #include <linux/device.h>
>>   #include <linux/export.h>
>>   #include <linux/math64.h>
>> +#include <linux/mfd/core.h>
>>   #include <linux/mfd/zl3073x.h>
>>   #include <linux/module.h>
>>   #include <linux/netlink.h>
>> @@ -755,6 +756,14 @@ static void zl3073x_devlink_unregister(void *ptr)
>>   	devlink_unregister(ptr);
>>   }
>>   
>> +static const struct mfd_cell zl3073x_dpll_cells[] = {
>> +	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 0),
>> +	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 1),
>> +	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 2),
>> +	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 3),
>> +	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 4),
>> +};
> 
> What other devices / subsystems will be involved when this is finished?

Lee, btw. I noticed from another discussion that you mentioned that
mfd_cell->id should not be used outside MFD.

My sub-drivers uses this to get DPLL channel number that should be used
for the particular sub-device.

E.g.
1) MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 2);
2) MFD_CELL_BASIC("zl3073x-phc", NULL, NULL, 0, 3);

In these cases dpll_zl3073x sub-driver will use DPLL channel 2 for this
DPLL sub-device and ptp_zl3073x sub-driver will use DPLL channel 3 for
this PHC sub-device.

platform_device->id cannot be used for this purpose in conjunction with
PLATFORM_DEVID_AUTO as that ->id can be arbitrary.

So if I cannot use mfd_cell->id what should I use for that case?
Platform data per cell with e.g. the DPLL channel number?

Thanks for advices...

Ivan

Re: [PATCH net-next v6 8/8] mfd: zl3073x: Register DPLL sub-device during init

Posted by Lee Jones 9 months, 1 week ago

On Fri, 02 May 2025, Ivan Vecera wrote:

> 
> 
> On 01. 05. 25 3:22 odp., Lee Jones wrote:
> > On Wed, 30 Apr 2025, Ivan Vecera wrote:
> > 
> > > Register DPLL sub-devices to expose the functionality provided
> > > by ZL3073x chip family. Each sub-device represents one of
> > > the available DPLL channels.
> > > 
> > > Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> > > ---
> > > v4->v6:
> > > * no change
> > > v3->v4:
> > > * use static mfd cells
> > > ---
> > >   drivers/mfd/zl3073x-core.c | 19 +++++++++++++++++++
> > >   1 file changed, 19 insertions(+)
> > > 
> > > diff --git a/drivers/mfd/zl3073x-core.c b/drivers/mfd/zl3073x-core.c
> > > index 050dc57c90c3..3e665cdf228f 100644
> > > --- a/drivers/mfd/zl3073x-core.c
> > > +++ b/drivers/mfd/zl3073x-core.c
> > > @@ -7,6 +7,7 @@
> > >   #include <linux/device.h>
> > >   #include <linux/export.h>
> > >   #include <linux/math64.h>
> > > +#include <linux/mfd/core.h>
> > >   #include <linux/mfd/zl3073x.h>
> > >   #include <linux/module.h>
> > >   #include <linux/netlink.h>
> > > @@ -755,6 +756,14 @@ static void zl3073x_devlink_unregister(void *ptr)
> > >   	devlink_unregister(ptr);
> > >   }
> > > +static const struct mfd_cell zl3073x_dpll_cells[] = {
> > > +	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 0),
> > > +	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 1),
> > > +	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 2),
> > > +	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 3),
> > > +	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 4),
> > > +};
> > 
> > What other devices / subsystems will be involved when this is finished?
> 
> Lee, btw. I noticed from another discussion that you mentioned that
> mfd_cell->id should not be used outside MFD.
> 
> My sub-drivers uses this to get DPLL channel number that should be used
> for the particular sub-device.
> 
> E.g.
> 1) MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 2);
> 2) MFD_CELL_BASIC("zl3073x-phc", NULL, NULL, 0, 3);
> 
> In these cases dpll_zl3073x sub-driver will use DPLL channel 2 for this
> DPLL sub-device and ptp_zl3073x sub-driver will use DPLL channel 3 for
> this PHC sub-device.
> 
> platform_device->id cannot be used for this purpose in conjunction with
> PLATFORM_DEVID_AUTO as that ->id can be arbitrary.
> 
> So if I cannot use mfd_cell->id what should I use for that case?
> Platform data per cell with e.g. the DPLL channel number?

Yes, using the device ID for anything other than enumeration is a hack.

Channel numbers and the like should be passed as platform data.

-- 
Lee Jones [李琼斯]

Re: [PATCH net-next v6 8/8] mfd: zl3073x: Register DPLL sub-device during init

Posted by Ivan Vecera 9 months, 1 week ago

On 07. 05. 25 1:06 odp., Lee Jones wrote:
> On Fri, 02 May 2025, Ivan Vecera wrote:
> 
>>
>>
>> On 01. 05. 25 3:22 odp., Lee Jones wrote:
>>> On Wed, 30 Apr 2025, Ivan Vecera wrote:
>>>
>>>> Register DPLL sub-devices to expose the functionality provided
>>>> by ZL3073x chip family. Each sub-device represents one of
>>>> the available DPLL channels.
>>>>
>>>> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
>>>> ---
>>>> v4->v6:
>>>> * no change
>>>> v3->v4:
>>>> * use static mfd cells
>>>> ---
>>>>    drivers/mfd/zl3073x-core.c | 19 +++++++++++++++++++
>>>>    1 file changed, 19 insertions(+)
>>>>
>>>> diff --git a/drivers/mfd/zl3073x-core.c b/drivers/mfd/zl3073x-core.c
>>>> index 050dc57c90c3..3e665cdf228f 100644
>>>> --- a/drivers/mfd/zl3073x-core.c
>>>> +++ b/drivers/mfd/zl3073x-core.c
>>>> @@ -7,6 +7,7 @@
>>>>    #include <linux/device.h>
>>>>    #include <linux/export.h>
>>>>    #include <linux/math64.h>
>>>> +#include <linux/mfd/core.h>
>>>>    #include <linux/mfd/zl3073x.h>
>>>>    #include <linux/module.h>
>>>>    #include <linux/netlink.h>
>>>> @@ -755,6 +756,14 @@ static void zl3073x_devlink_unregister(void *ptr)
>>>>    	devlink_unregister(ptr);
>>>>    }
>>>> +static const struct mfd_cell zl3073x_dpll_cells[] = {
>>>> +	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 0),
>>>> +	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 1),
>>>> +	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 2),
>>>> +	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 3),
>>>> +	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 4),
>>>> +};
>>>
>>> What other devices / subsystems will be involved when this is finished?
>>
>> Lee, btw. I noticed from another discussion that you mentioned that
>> mfd_cell->id should not be used outside MFD.
>>
>> My sub-drivers uses this to get DPLL channel number that should be used
>> for the particular sub-device.
>>
>> E.g.
>> 1) MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 2);
>> 2) MFD_CELL_BASIC("zl3073x-phc", NULL, NULL, 0, 3);
>>
>> In these cases dpll_zl3073x sub-driver will use DPLL channel 2 for this
>> DPLL sub-device and ptp_zl3073x sub-driver will use DPLL channel 3 for
>> this PHC sub-device.
>>
>> platform_device->id cannot be used for this purpose in conjunction with
>> PLATFORM_DEVID_AUTO as that ->id can be arbitrary.
>>
>> So if I cannot use mfd_cell->id what should I use for that case?
>> Platform data per cell with e.g. the DPLL channel number?
> 
> Yes, using the device ID for anything other than enumeration is a hack.
> 
> Channel numbers and the like should be passed as platform data.

OK, I will send v7 quickly.

Ivan

Re: [PATCH net-next v6 8/8] mfd: zl3073x: Register DPLL sub-device during init

Posted by Ivan Vecera 9 months, 1 week ago

On 01. 05. 25 3:22 odp., Lee Jones wrote:
> On Wed, 30 Apr 2025, Ivan Vecera wrote:
> 
>> Register DPLL sub-devices to expose the functionality provided
>> by ZL3073x chip family. Each sub-device represents one of
>> the available DPLL channels.
>>
>> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
>> ---
>> v4->v6:
>> * no change
>> v3->v4:
>> * use static mfd cells
>> ---
>>   drivers/mfd/zl3073x-core.c | 19 +++++++++++++++++++
>>   1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/mfd/zl3073x-core.c b/drivers/mfd/zl3073x-core.c
>> index 050dc57c90c3..3e665cdf228f 100644
>> --- a/drivers/mfd/zl3073x-core.c
>> +++ b/drivers/mfd/zl3073x-core.c
>> @@ -7,6 +7,7 @@
>>   #include <linux/device.h>
>>   #include <linux/export.h>
>>   #include <linux/math64.h>
>> +#include <linux/mfd/core.h>
>>   #include <linux/mfd/zl3073x.h>
>>   #include <linux/module.h>
>>   #include <linux/netlink.h>
>> @@ -755,6 +756,14 @@ static void zl3073x_devlink_unregister(void *ptr)
>>   	devlink_unregister(ptr);
>>   }
>>   
>> +static const struct mfd_cell zl3073x_dpll_cells[] = {
>> +	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 0),
>> +	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 1),
>> +	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 2),
>> +	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 3),
>> +	MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 4),
>> +};
> 
> What other devices / subsystems will be involved when this is finished?
> 
PHC/PTP driver and in future GPIO controller.

I'm adding here only DPLL for now as it is finished and ready
(part2)... PTP driver is now in progress and GPIO is in planning phase.

Ivan