Re: [PATCH v2 3/4] pwm: atmel: add support for SAM9X60's PWM controller

[<Claudiu.Beznea@microchip.com>]

On 21.02.2019 22:45, Uwe Kleine-König wrote:
> On Tue, Feb 19, 2019 at 10:09:00AM +0000, Claudiu.Beznea@microchip.com wrote:
>> From: Claudiu Beznea <claudiu.beznea@microchip.com>
>>
>> Add support for SAM9X60's PWM controller.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>> ---
>>  drivers/pwm/pwm-atmel.c | 19 +++++++++++++++++++
>>  1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
>> index 647d063562db..229cedb02770 100644
>> --- a/drivers/pwm/pwm-atmel.c
>> +++ b/drivers/pwm/pwm-atmel.c
>> @@ -52,6 +52,8 @@
>>  
>>  /* Only the LSB 16 bits are significant. */
>>  #define PWM_MAXV1_PRD		0xFFFF
>> +/* All 32 bits are significant. */
>> +#define PWM_MAXV2_PRD		0xFFFFFFFF
>>  #define PRD_MAXV1_PRES		10
>>  
>>  struct atmel_pwm_registers {
>> @@ -311,6 +313,20 @@ static const struct atmel_pwm_data atmel_pwm_data_v2 = {
>>  	},
>>  };
>>  
>> +static const struct atmel_pwm_data atmel_pwm_data_v3 = {
> 
> Does it make more sense to call this ..._sam9x60 to match the
> compatible? (If yes, patch 1 should be changed accordingly.)

It could be changed, yep.

> 
> I wonder how the naming of the defines is chosen given that pwm_data_v3
> is the first that needs PWM_MAXV2_PRD. Looks inconsistent.

I know... I'm aware of that. The thing is controllers may differ with
regards to in-flight duty update and now there is this new difference w/
regards to counters size.

Renaming the objects of type atmel_pwm_data in something like
atmel_pwm_data_<chip-name> as you suggested before would make things clear
for you?

Thank you,
Claudiu Beznea

> 
> Best regards
> Uwe
>