Hi Komal,
This one ended up in my spam box once again, although with a lower
score (using the proper type for the attachement seems to have helped.)
Maybe try with a shorter list of recipients next time, and add the name
of people before their address.
> Attached the updated patch as per the review comments on #3 patch.
>
> Please consider it for the inclusion.
Looks good, I'll take it. One remaining objection:
> +static int omap_i2c_get_clocks(struct omap_i2c_dev *dev)
> +{
> + if (cpu_is_omap16xx() || cpu_is_omap24xx()) {
> + dev->iclk = clk_get(dev->dev, "i2c_ick");
> + if (IS_ERR(dev->iclk)) {
> + dev->iclk = NULL;
> + return -ENODEV;
> + }
> + }
> +
> + dev->fclk = clk_get(dev->dev, "i2c_fck");
> + if (IS_ERR(dev->fclk)) {
> + if (dev->iclk != NULL) {
> + clk_put(dev->iclk);
> + dev->iclk = NULL;
> + return -ENODEV;
I think this return shouldn't be there.
> + }
> + dev->fclk = NULL;
> + return -ENODEV;
> + }
> +
> + return 0;
> +}
I'll fix it myself if you agree, so that you don't have to resend a
patch. Thanks for the good work! I'll send the patch upstream at the
end of the week, so it should show in -mm soon.
--
Jean Delvare
--- Jean Delvare <[email protected]> wrote:
> Hi Komal,
>
> This one ended up in my spam box once again, although with a lower
> score (using the proper type for the attachement seems to have
> helped.)
> Maybe try with a shorter list of recipients next time, and add the
> name
> of people before their address.
I hope this doesn't end up in spam box ;).
>
> > Attached the updated patch as per the review comments on #3 patch.
> >
> > Please consider it for the inclusion.
>
> Looks good, I'll take it. One remaining objection:
>
> > +static int omap_i2c_get_clocks(struct omap_i2c_dev *dev)
> > +{
> > + if (cpu_is_omap16xx() || cpu_is_omap24xx()) {
> > + dev->iclk = clk_get(dev->dev, "i2c_ick");
> > + if (IS_ERR(dev->iclk)) {
> > + dev->iclk = NULL;
> > + return -ENODEV;
> > + }
> > + }
> > +
> > + dev->fclk = clk_get(dev->dev, "i2c_fck");
> > + if (IS_ERR(dev->fclk)) {
> > + if (dev->iclk != NULL) {
> > + clk_put(dev->iclk);
> > + dev->iclk = NULL;
> > + return -ENODEV;
>
> I think this return shouldn't be there.
>
> > + }
> > + dev->fclk = NULL;
> > + return -ENODEV;
> > + }
> > +
> > + return 0;
> > +}
>
> I'll fix it myself if you agree, so that you don't have to resend a
> patch. Thanks for the good work! I'll send the patch upstream at the
> end of the week, so it should show in -mm soon.
I am fine with that change. Thanx for the review.
Signed-off-by: Komal Shah <[email protected]>
---Komal Shah
http://komalshah.blogspot.com/
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com