diff mbox series

[6/8] counter: stm32-lptimer-cnt: add support for stm32mp25

Message ID 20250224180150.3689638-7-fabrice.gasnier@foss.st.com (mailing list archive)
State New
Headers show
Series Add STM32MP25 LPTIM support: MFD, PWM, IIO, counter, clocksource | expand

Commit Message

Fabrice Gasnier Feb. 24, 2025, 6:01 p.m. UTC
Add support for STM32MP25 SoC. Use newly introduced compatible to handle
this new HW variant, even if no major change is expected on the counter
driver.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
---
 drivers/counter/stm32-lptimer-cnt.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Krzysztof Kozlowski Feb. 25, 2025, 12:02 p.m. UTC | #1
On Mon, Feb 24, 2025 at 07:01:48PM +0100, Fabrice Gasnier wrote:
> Add support for STM32MP25 SoC. Use newly introduced compatible to handle
> this new HW variant, even if no major change is expected on the counter
> driver.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
> ---
>  drivers/counter/stm32-lptimer-cnt.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/counter/stm32-lptimer-cnt.c b/drivers/counter/stm32-lptimer-cnt.c
> index b249c8647639..a5dce017c37b 100644
> --- a/drivers/counter/stm32-lptimer-cnt.c
> +++ b/drivers/counter/stm32-lptimer-cnt.c
> @@ -508,6 +508,7 @@ static SIMPLE_DEV_PM_OPS(stm32_lptim_cnt_pm_ops, stm32_lptim_cnt_suspend,
>  
>  static const struct of_device_id stm32_lptim_cnt_of_match[] = {
>  	{ .compatible = "st,stm32-lptimer-counter", },
> +	{ .compatible = "st,stm32mp25-lptimer-counter", },

So fully compatible? Why this change then?

Best regards,
Krzysztof
Fabrice Gasnier Feb. 25, 2025, 2:58 p.m. UTC | #2
On 2/25/25 13:02, Krzysztof Kozlowski wrote:
> On Mon, Feb 24, 2025 at 07:01:48PM +0100, Fabrice Gasnier wrote:
>> Add support for STM32MP25 SoC. Use newly introduced compatible to handle
>> this new HW variant, even if no major change is expected on the counter
>> driver.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
>> ---
>>  drivers/counter/stm32-lptimer-cnt.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/counter/stm32-lptimer-cnt.c b/drivers/counter/stm32-lptimer-cnt.c
>> index b249c8647639..a5dce017c37b 100644
>> --- a/drivers/counter/stm32-lptimer-cnt.c
>> +++ b/drivers/counter/stm32-lptimer-cnt.c
>> @@ -508,6 +508,7 @@ static SIMPLE_DEV_PM_OPS(stm32_lptim_cnt_pm_ops, stm32_lptim_cnt_suspend,
>>  
>>  static const struct of_device_id stm32_lptim_cnt_of_match[] = {
>>  	{ .compatible = "st,stm32-lptimer-counter", },
>> +	{ .compatible = "st,stm32mp25-lptimer-counter", },
> 
> So fully compatible? Why this change then?

Hi Krzysztof,

I should have mentioned it in the commit message:

Currently, same feature list as on STM32MP1x is supported.
New capture input stage is now available in the hardware. It's not added
yet to the driver.

The Low Power Timer (LPTIM) hardware isn't fully backward compatible,
hence add a new compatible to properly support it.

Best regards,
Fabrice

> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Feb. 26, 2025, 7:49 a.m. UTC | #3
On 25/02/2025 15:58, Fabrice Gasnier wrote:
> On 2/25/25 13:02, Krzysztof Kozlowski wrote:
>> On Mon, Feb 24, 2025 at 07:01:48PM +0100, Fabrice Gasnier wrote:
>>> Add support for STM32MP25 SoC. Use newly introduced compatible to handle
>>> this new HW variant, even if no major change is expected on the counter
>>> driver.
>>>
>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
>>> ---
>>>  drivers/counter/stm32-lptimer-cnt.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/counter/stm32-lptimer-cnt.c b/drivers/counter/stm32-lptimer-cnt.c
>>> index b249c8647639..a5dce017c37b 100644
>>> --- a/drivers/counter/stm32-lptimer-cnt.c
>>> +++ b/drivers/counter/stm32-lptimer-cnt.c
>>> @@ -508,6 +508,7 @@ static SIMPLE_DEV_PM_OPS(stm32_lptim_cnt_pm_ops, stm32_lptim_cnt_suspend,
>>>  
>>>  static const struct of_device_id stm32_lptim_cnt_of_match[] = {
>>>  	{ .compatible = "st,stm32-lptimer-counter", },
>>> +	{ .compatible = "st,stm32mp25-lptimer-counter", },
>>
>> So fully compatible? Why this change then?
> 
> Hi Krzysztof,
> 
> I should have mentioned it in the commit message:
> 
> Currently, same feature list as on STM32MP1x is supported.
> New capture input stage is now available in the hardware. It's not added
> yet to the driver.

Which is the exact meaning of compatible. Express it properly in
bindings and driver.

> 
> The Low Power Timer (LPTIM) hardware isn't fully backward compatible,

How so? How can it work then with above ID table?


Best regards,
Krzysztof
Fabrice Gasnier Feb. 26, 2025, 6:16 p.m. UTC | #4
On 2/26/25 08:49, Krzysztof Kozlowski wrote:
> On 25/02/2025 15:58, Fabrice Gasnier wrote:
>> On 2/25/25 13:02, Krzysztof Kozlowski wrote:
>>> On Mon, Feb 24, 2025 at 07:01:48PM +0100, Fabrice Gasnier wrote:
>>>> Add support for STM32MP25 SoC. Use newly introduced compatible to handle
>>>> this new HW variant, even if no major change is expected on the counter
>>>> driver.
>>>>
>>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
>>>> ---
>>>>  drivers/counter/stm32-lptimer-cnt.c | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/counter/stm32-lptimer-cnt.c b/drivers/counter/stm32-lptimer-cnt.c
>>>> index b249c8647639..a5dce017c37b 100644
>>>> --- a/drivers/counter/stm32-lptimer-cnt.c
>>>> +++ b/drivers/counter/stm32-lptimer-cnt.c
>>>> @@ -508,6 +508,7 @@ static SIMPLE_DEV_PM_OPS(stm32_lptim_cnt_pm_ops, stm32_lptim_cnt_suspend,
>>>>  
>>>>  static const struct of_device_id stm32_lptim_cnt_of_match[] = {
>>>>  	{ .compatible = "st,stm32-lptimer-counter", },
>>>> +	{ .compatible = "st,stm32mp25-lptimer-counter", },
>>>
>>> So fully compatible? Why this change then?
>>
>> Hi Krzysztof,
>>
>> I should have mentioned it in the commit message:
>>
>> Currently, same feature list as on STM32MP1x is supported.
>> New capture input stage is now available in the hardware. It's not added
>> yet to the driver.
> 
> Which is the exact meaning of compatible. Express it properly in
> bindings and driver.

Could you please just clarify here, do you suggest to better explain it
in the commit message, both in bindings and driver ?
Or do you mean something else ?

There's one more thing here, I haven't mentioned earlier, which is
specific to the STM32MP25 SoC family (not only to the LPTimer revision).

The LPTimer has interconnect signals throughout all the SoC. It's listed
in the "Peripheral interconnects" section of the Reference Manual (RM0457).
Here, simply reading the LPTimer revision/identification register
wouldn't be enough, as it specific to the SoC. Upcoming STM32MP21 SoC
family will have a different list of inputs/outpus internally in the SoC
(but same controller revision).

So here I intend to keep the specific compatible
"st,stm32mp25-lptimer-counter", to be used even if not yet implemented
by the driver.

I'll try to explain it better in next revision.

Thanks for reviewing,
Best Regards,
Fabrice

> 
>>
>> The Low Power Timer (LPTIM) hardware isn't fully backward compatible,
> 
> How so? How can it work then with above ID table?
> 
> 
> Best regards,
> Krzysztof
diff mbox series

Patch

diff --git a/drivers/counter/stm32-lptimer-cnt.c b/drivers/counter/stm32-lptimer-cnt.c
index b249c8647639..a5dce017c37b 100644
--- a/drivers/counter/stm32-lptimer-cnt.c
+++ b/drivers/counter/stm32-lptimer-cnt.c
@@ -508,6 +508,7 @@  static SIMPLE_DEV_PM_OPS(stm32_lptim_cnt_pm_ops, stm32_lptim_cnt_suspend,
 
 static const struct of_device_id stm32_lptim_cnt_of_match[] = {
 	{ .compatible = "st,stm32-lptimer-counter", },
+	{ .compatible = "st,stm32mp25-lptimer-counter", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, stm32_lptim_cnt_of_match);