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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
