Hi Andrew, Thanks for the review. > On Thu, Nov 13, 2014 at 5:55 AM, Sai Masarapu <Sai.Masarapu@xxxxxxxxxx> wrote: >> On Wed, Nov 12, 2014 at 4:37 AM, <Naidu.Tellapati@xxxxxxxxxx> wrote: >>>> From: Naidu Tellapati <Naidu.Tellapati@xxxxxxxxxx> >>>> Signed-off-by: Naidu Tellapati <Naidu.Tellapati@xxxxxxxxxx> >>>> Signed-off-by: Sai Masarapu <Sai.Masarapu@xxxxxxxxxx> >>>> +#define CR_PERIP_PWM_PDM_CONTROL 0x0140 /* PWM and PDM ctrl register */ >>>> +#define PWM_PDM_CH_CTRL_SHIFT(ch) (ch * 4) >>>> + >>>> +#define NUM_PWM 4 >>>> +#define SUB_DIV0 1 >>>> +#define SUB_DIV1 2 >>> +#define NO_SUB_DIV 0 >>>> +#define SUB_DIV0_DIV1 3 >>>> +#define MIN_TMBASE_STEPS 1 >>>> +#define MAX_TMBASE_STEPS 65536 >>>> +#define MIN_DUTY_STEPS 0 >>>> +#define MAX_DUTY_STEPS 65535 >> >>> Please place these #defines next to their corresponding register or register field #define. >> >> Defined Macros are not related to register fields. > Really? The SUB_DIV #defines are possible values for the DIVIDER > field in CR_PWM_CTRL_CFG, are they not? And I believe the TMBASE and > DUTY #defines specify the range of values for the DUTYCYCLE and > TIMEBASE fields of the CR_PWM*_CFG registers. Sai might have overlooked these fields. We will address this in the next patch set. Regards, Naidu. -- 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