On Tue, Jan 21, 2014 at 08:35:47AM -0800, Guenter Roeck wrote:
> On Tue, Jan 21, 2014 at 10:26:07AM -0300, Ezequiel Garcia wrote:
> > This commit adds an orion_watchdog_data structure to holda compatible-data
> 
>       s/holda/hold/
> 

Yup.

> > information. This allows to remove the driver-wide definition and to
> > future add support for multiple compatible-strings.
> 
> Maybe "and to be able to add support for multiple compatible-strings in the
> future" ?
> 

Yes, sounds better.

[..]
> >  
> >  #define WDT_MAX_CYCLE_COUNT        0xffffffff
> >  #define WDT_IN_USE         0
> >  #define WDT_OK_TO_CLOSE            1
> >  
> While looking at the new defines below, I noticed that WDT_IN_USE and
> WDT_OK_TO_CLOSE are not used (anymore ?) and thus should be removed
> (separate patch, though).
> 

OK, let's clean this.

> > -#define WDT_RESET_OUT_EN   BIT(1)
> > +#define WDT_A370_RATIO_MASK(v)     ((v) << 16)
> > +#define WDT_A370_RATIO_SHIFT       5
> > +#define WDT_A370_RATIO             (1 << WDT_A370_RATIO_SHIFT)
> > +
> > +#define WDT_AXP_FIXED_ENABLE_BIT BIT(10)
> >  
> Those new defines are not used. They should be introduced if and when used.
> 

Hm... maybe after dozens and dozens of rebases these macros sneaked here?

[..]
> >  
> > +   match = of_match_device(orion_wdt_of_match_table, &pdev->dev);
> > +   if (!match)
> > +           /* Default legacy match */
> > +           match = &orion_wdt_of_match_table[0];
> > +
> >     dev->wdt.info = &orion_wdt_info;
> >     dev->wdt.ops = &orion_wdt_ops;
> >     dev->wdt.min_timeout = 1;
> > +   dev->data = (struct orion_watchdog_data *)match->data;
> 
> match->data is a void *, so the typecast should not be needed here.
> You might want to declare 'data' to be of type
> 'const struct orion_watchdog_data *' because otherwise you loose the
> 'const' attribute of match->data (which may be the reason for the typecast ?).
> 

Done.

Thanks a lot for reviewing!
-- 
Ezequiel GarcĂ­a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to