On Sunday 21 of July 2013 21:50:47 Sylwester Nawrocki wrote:
> On 07/20/2013 02:04 AM, Tomasz Figa wrote:
> > This patch introduces new Samsung PWM driver, which uses Samsung
> > PWM/timer master driver to control shared parts of the hardware.
> >
> > Signed-off-by: Tomasz Figa<tomasz.figa@xxxxxxxxx>
> > ---
> >
> > drivers/pwm/Makefile | 1 +
> > drivers/pwm/pwm-samsung.c | 606
> > ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 607
> > insertions(+)
> > create mode 100644 drivers/pwm/pwm-samsung.c
>
> [...]
>
> > +static const struct samsung_pwm_variant s3c64xx_variant = {
> > + .bits = 32,
> > + .div_base = 0,
>
> Initialization to 0 could be omitted, since it is implicit.
Well, I'd prefer keeping this as is, for clarity. It improves code
readability, because you don't have to look at struct definition to see
all the fields.
> > + .has_tint_cstat = true,
> > + .tclk_mask = BIT(7) | BIT(6) | BIT(5),
> > +};
> > +
> > +static const struct samsung_pwm_variant s5p64x0_variant = {
> > + .bits = 32,
> > + .div_base = 0,
>
> Ditto.
>
> > + .has_tint_cstat = true,
> > +};
> > +
> > +static const struct samsung_pwm_variant s5p_variant = {
> > + .bits = 32,
> > + .div_base = 0,
>
> Ditto.
>
> > + .has_tint_cstat = true,
> > + .tclk_mask = BIT(5),
> > +};
> > +
>
> [...]
>
> > +static int pwm_samsung_remove(struct platform_device *pdev)
> > +{
> > + struct samsung_pwm_chip *chip = platform_get_drvdata(pdev);
> > + int ret;
> > +
> > + ret = pwmchip_remove(&chip->chip);
> > + if (ret < 0)
> > + return ret;
>
> Since return value of the remove() callback is happily ignored by
> the driver core I think there is no point in returning an error
> here like this. Wouldn't it be more sensible to just call all the
> cleanup functions unconditionally ? At least potentially wrong state
> of the clock could be avoided.
IMHO handling the errors correctly here is more future-proof. Driver core
might be modified to handle errors in future and this driver will not need
to be modified.
Best regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
