[LINUX PATCH v2 1/3] clocksource: timer-cadence-ttc: Do not probe TTC device configured as PWM

[Sayyed, Mubin]

Hi,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski at linaro.org>
> Sent: Friday, November 24, 2023 5:06 PM
> To: Sayyed, Mubin <mubin.sayyed at amd.com>
> Cc: linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org;
> devicetree at vger.kernel.org; linux-pwm at vger.kernel.org; git (AMD-Xilinx)
> <git at amd.com>; mubin10 at gmail.com; krzysztof.kozlowski+dt at linaro.org;
> u.kleine-koenig at pengutronix.de; thierry.reding at gmail.com;
> robh+dt at kernel.org; conor+dt at kernel.org; tglx at linutronix.de;
> daniel.lezcano at linaro.org; Simek, Michal <michal.simek at amd.com>
> Subject: Re: [LINUX PATCH v2 1/3] clocksource: timer-cadence-ttc: Do not probe
> TTC device configured as PWM
> 
> On 24/11/2023 12:03, Sayyed, Mubin wrote:
> > Hi Krzysztof,
> >
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzysztof.kozlowski at linaro.org>
> >> Sent: Wednesday, November 15, 2023 5:41 PM
> >> To: Sayyed, Mubin <mubin.sayyed at amd.com>
> >> Cc: linux-arm-kernel at lists.infradead.org;
> >> linux-kernel at vger.kernel.org; devicetree at vger.kernel.org;
> >> linux-pwm at vger.kernel.org; git (AMD-Xilinx) <git at amd.com>;
> >> mubin10 at gmail.com; krzysztof.kozlowski+dt at linaro.org;
> >> u.kleine-koenig at pengutronix.de; thierry.reding at gmail.com;
> >> robh+dt at kernel.org; conor+dt at kernel.org; tglx at linutronix.de;
> >> daniel.lezcano at linaro.org; Simek, Michal <michal.simek at amd.com>
> >> Subject: Re: [LINUX PATCH v2 1/3] clocksource: timer-cadence-ttc: Do
> >> not probe TTC device configured as PWM
> >>
> >> On 15/11/2023 06:55, Sayyed, Mubin wrote:
> >>>>> +	/*
> >>>>> +	 * If pwm-cells property is present in TTC node,
> >>>>> +	 * it would be treated as PWM device.
> >>>>> +	 */
> >>>>> +	if (of_property_read_bool(timer, "#pwm-cells"))
> >>>>> +		return -ENODEV;
> >>>>
> >>>> You will introduce dmesg errors, so regressions.
> >>>>
> >>> [Mubin]: I will change it to "return 0" to avoid dmesg errors.
> >>
> >> No, because solution is wrong.
> >>
> >>>
> >>>> This does not look right. What you want is to bind one device
> >>>> driver and choose different functionality based on properties.
> >>> [Mubin]:  I am doing it based on earlier discussion related to AXI
> >>> Timer PWM
> >> driver.  It was suggested to use #pwm-cells property for identifying
> >> role of
> >> device(PWM/clocksource) https://lore.kernel.org/linux-
> >> devicetree/20210513021631.GA878860 at robh.at.kernel.org/.
> >>
> >> You are mixing bindings with driver. I said here about driver and yes
> >> - you must use pwm-cells to differentiate that. It's obvious.
> >>
> >> So again, one driver binding.
> > [Mubin]: I will explore whether mfd framework can be used to handle this.
> 
> You do not need MFD for this, because you do not have a really MFD. This is just
> one device, so I expect here one driver. Why do you need multiple drivers (which
> also would solve that problem but why?)?
Cadence TTC IP can be used as timer(clocksource/clockevent) and PWM device.
We have drivers/clocksource/timer-cadence-ttc.c for clocksource/clockevent functionality. 
New driver for PWM functionality will be added to drivers/pwm/pwm-cadence.c (3/3 of this
Series).  In given SoC,  multiple instances of TTC IP are possible(ZynqMP  Ultrscale SoC has 4
Instances), few of them could be configured as clocksource/clockevent devices and others
as PWM ones. So,  cloksource as well as PWM drivers for cadence TTC IP would be enabled in 
the kernel. 

Now in this scenario, each TTC device would be matching with 2 drivers, clocksource and PWM, since
compatible string is same.  If I don’t add #pwm-cells checking in clocksource driver and return 
-ENODEV based on that, each device would always bind with clocksource driver. PWM driver 
would never probe since clocksource driver probes ahead of PWM one in probing order.

I am exploring mfd to deal with said scenario. Do you see any better way to handle this? 

Thanks,
Mubin

> 
> Best regards,
> Krzysztof