lkml.org 
[lkml]   [2014]   [Feb]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 01/10] mfd: Add TI LMU driver
Hi Lee,

On 02/17/2014 06:57 PM, Lee Jones wrote:
>> +static const struct resource lm3633_effect_resources[] = {
>> + {
>> + .name = LM3633_EFFECT_BL0_RAMPUP,
>> + .flags = IORESOURCE_REG,
>> + .start = LM3633_EFFECT_REGISTER(BL0_RAMPUP),
>> + },
>> + {
>> + .name = LM3633_EFFECT_BL0_RAMPDOWN,
>> + .flags = IORESOURCE_REG,
>> + .start = LM3633_EFFECT_REGISTER(BL0_RAMPDN),
>> + },
>> + {
>> + .name = LM3633_EFFECT_BL1_RAMPUP,
>> + .flags = IORESOURCE_REG,
>> + .start = LM3633_EFFECT_REGISTER(BL1_RAMPUP),
>> + },
>> + {
>> + .name = LM3633_EFFECT_BL1_RAMPDOWN,
>> + .flags = IORESOURCE_REG,
>> + .start = LM3633_EFFECT_REGISTER(BL1_RAMPDN),
>> + },
>> + {
>> + .name = LM3633_EFFECT_PTN_DELAY,
>> + .flags = IORESOURCE_REG,
>> + .start = LM3633_EFFECT_REGISTER(DELAY),
>> + },
>> + {
>> + .name = LM3633_EFFECT_PTN_HIGHTIME,
>> + .flags = IORESOURCE_REG,
>> + .start = LM3633_EFFECT_REGISTER(HIGHTIME),
>> + },
>> + {
>> + .name = LM3633_EFFECT_PTN_LOWTIME,
>> + .flags = IORESOURCE_REG,
>> + .start = LM3633_EFFECT_REGISTER(LOWTIME),
>> + },
>> + {
>> + .name = LM3633_EFFECT_PTN0_RAMPUP,
>> + .flags = IORESOURCE_REG,
>> + .start = LM3633_EFFECT_REGISTER(PTN0_RAMPUP),
>> + },
>> + {
>> + .name = LM3633_EFFECT_PTN0_RAMPDOWN,
>> + .flags = IORESOURCE_REG,
>> + .start = LM3633_EFFECT_REGISTER(PTN0_RAMPDN),
>> + },
>> + {
>> + .name = LM3633_EFFECT_PTN1_RAMPUP,
>> + .flags = IORESOURCE_REG,
>> + .start = LM3633_EFFECT_REGISTER(PTN1_RAMPUP),
>> + },
>> + {
>> + .name = LM3633_EFFECT_PTN1_RAMPDOWN,
>> + .flags = IORESOURCE_REG,
>> + .start = LM3633_EFFECT_REGISTER(PTN1_RAMPDN),
>> + },
>> + {
>> + .name = LM3633_EFFECT_PTN_LOWBRT,
>> + .flags = IORESOURCE_REG,
>> + .start = LM3633_EFFECT_REGISTER(LOWBRT),
>> + },
>> + {
>> + .name = LM3633_EFFECT_PTN_HIGHBRT,
>> + .flags = IORESOURCE_REG,
>> + .start = LM3633_EFFECT_REGISTER(HIGHBRT),
>> + },
>> +};
>
> Can you define a MACRO to do all of these as one liners?

Yes, resource definitions will be replaced by simple macro,
LMU_EFFECT_RESOURCE().

For example,

#define LMU_EFFECT_RESOURCE(chip, effect) \
{ \
.name = chip##_EFFECT_##effect, \
.flags = IORESOURCE_REG, \
.start = LMU_EFFECT_REGISTER(chip, effect), \
}

static const struct resource lm3633_effect_resources[] = {
LMU_EFFECT_RESOURCE(LM3633, BL0_RAMPUP),
LMU_EFFECT_RESOURCE(LM3633, BL0_RAMPDN),
LMU_EFFECT_RESOURCE(LM3633, BL1_RAMPUP),
LMU_EFFECT_RESOURCE(LM3633, BL1_RAMPDN),
LMU_EFFECT_RESOURCE(LM3633, PTN_DELAY),
LMU_EFFECT_RESOURCE(LM3633, PTN_HIGHTIME),
LMU_EFFECT_RESOURCE(LM3633, PTN_LOWTIME),
LMU_EFFECT_RESOURCE(LM3633, PTN0_RAMPUP),
LMU_EFFECT_RESOURCE(LM3633, PTN0_RAMPDN),
LMU_EFFECT_RESOURCE(LM3633, PTN1_RAMPUP),
LMU_EFFECT_RESOURCE(LM3633, PTN1_RAMPDN),
LMU_EFFECT_RESOURCE(LM3633, PTN_LOWBRT),
LMU_EFFECT_RESOURCE(LM3633, PTN_HIGHBRT),
};

static const struct resource lm3697_effect_resources[] = {
LMU_EFFECT_RESOURCE(LM3697, BL0_RAMPUP),
LMU_EFFECT_RESOURCE(LM3697, BL0_RAMPDN),
LMU_EFFECT_RESOURCE(LM3697, BL1_RAMPUP),
LMU_EFFECT_RESOURCE(LM3697, BL1_RAMPDN),
};

and so on.

> <snip>
>
>> +static int ti_lmu_parse_dt(struct device *dev, struct ti_lmu *lmu)
>> +{
>
> <snip>
>
>> + pdata->en_gpio = of_get_named_gpio(node, "ti,enable-gpio", 0);
>
> There is a global DT property for this already.

I've not found it yet, but I agree it looks like general property.
So I'll replace "ti,enable-gpio" with "ti,lmu-en-gpio".

>
>> +static int ti_lmu_probe(struct i2c_client *cl, const struct i2c_device_id *id)
>> +{
>
> <snip>
>
>> + lmu->id = id->driver_data;
>> + switch (lmu->id) {
>> + case LM3532:
>> + lmu_regmap_config.max_register = LM3532_MAX_REGISTERS;
>> + break;
>> + case LM3631:
>> + lmu_regmap_config.max_register = LM3631_MAX_REGISTERS;
>> + break;
>> + case LM3633:
>> + lmu_regmap_config.max_register = LM3633_MAX_REGISTERS;
>> + break;
>> + case LM3695:
>> + lmu_regmap_config.max_register = LM3695_MAX_REGISTERS;
>> + break;
>> + case LM3697:
>> + lmu_regmap_config.max_register = LM3697_MAX_REGISTERS;
>> + break;
>> + default:
>> + break;
>> + }
>
> As there are so many of these, it might be nicer to pull these out
> into a seperate function.

Lines of code will be moved to new function, ti_lmu_regmap_init().

Thanks for your review.

Best regards,
Milo



\
 
 \ /
  Last update: 2014-02-18 09:21    [W:1.914 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site