Re: [PATCH v8 1/2] PWM: atmel-pwm: add PWM controller driver

[Bo Shen]

Hi Thierry,

On 12/03/2013 11:09 AM, Bo Shen wrote:
> > > +        atmel_pwm->chip.of_xlate = of_pwm_xlate_with_flags;
> > > +        atmel_pwm->chip.of_pwm_n_cells = 3;
> > > +        atmel_pwm->chip.base = -1;
> > > +    } else {
> > > +        atmel_pwm->chip.base = pdev->id;
> >
> > That's not correct. The chip cannot be tied to pdev->id, because that ID
> > is the instance number of the device. So typically you would have
> > devices name like this:
> >
> >     atmel-pwm.0
> >     atmel-pwm.1
> >     ...
> >
> > Now, if you have that, then you won't be able to register the second
> > instance because the first instance will already have requested PWMs
> > 0-3, and setting .base to 1 will cause PWMs 1-4 to be requested, which
> > intersects with the range of the first instance.
> >
> > The same applies of course if you have other PWM controllers in the
> > system which have similar instance names.
> >
> > So the right thing to do here is to provide that number via platform
> > data so that platform code can define it, knowing in advance all ranges
> > for all other PWM controllers and thereby make sure there's no
> > intersection.
>
> OK, I will fix this.

After read deeply of PWM framework, for non device tree, I think we'd 
better let the PWM core to choose chip.base as device tree, while not 
pass a number through platform data to it. Or else, it will confuse the 
user to set the chip.base, must set it in correct value to avoid 
intersection. And, actually we won't use chip.base in driver itself.

Hi J,
  what's your opinion?

Best Regards,
Bo Shen

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/