lkml.org 
[lkml]   [2016]   [May]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCHv3] support for AD5820 camera auto-focus coil
From
Date


On 24.05.2016 12:04, Pavel Machek wrote:
> Hi!
>
>>> +static int ad5820_registered(struct v4l2_subdev *subdev)
>>> +{
>>> + struct ad5820_device *coil = to_ad5820_device(subdev);
>>> + struct i2c_client *client = v4l2_get_subdevdata(subdev);
>>> +
>>> + coil->vana = regulator_get(&client->dev, "VANA");
>>
>> devm_regulator_get()?
>
> I'd rather avoid devm_ here. Driver is simple enough to allow it.
>

Now thinking about it, what would happen here if regulator_get() returns
-EPROBE_DEFER? Wouldn't it be better to move regulator_get to the
probe() function, something like:

static int ad5820_probe(struct i2c_client *client,
const struct i2c_device_id *devid)
{
struct ad5820_device *coil;
int ret = 0;

coil = devm_kzalloc(sizeof(*coil), GFP_KERNEL);
if (coil == NULL)
return -ENOMEM;

coil->vana = devm_regulator_get(&client->dev, NULL);
if (IS_ERR(coil->vana)) {
ret = PTR_ERR(coil->vana);
if (ret != -EPROBE_DEFER)
dev_err(&client->dev, "could not get regulator for vana\n");
return ret;
}

mutex_init(&coil->power_lock);
...

with the appropriate changes to remove() because of the devm API usage.

>>> +#define AD5820_RAMP_MODE_LINEAR (0 << 3)
>>> +#define AD5820_RAMP_MODE_64_16 (1 << 3)
>>> +
>>> +struct ad5820_platform_data {
>>> + int (*set_xshutdown)(struct v4l2_subdev *subdev, int set);
>>> +};
>>> +
>>> +#define to_ad5820_device(sd) container_of(sd, struct ad5820_device, subdev)
>>> +
>>> +struct ad5820_device {
>>> + struct v4l2_subdev subdev;
>>> + struct ad5820_platform_data *platform_data;
>>> + struct regulator *vana;
>>> +
>>> + struct v4l2_ctrl_handler ctrls;
>>> + u32 focus_absolute;
>>> + u32 focus_ramp_time;
>>> + u32 focus_ramp_mode;
>>> +
>>> + struct mutex power_lock;
>>> + int power_count;
>>> +
>>> + int standby : 1;
>>> +};
>>> +
>>
>> The same for struct ad5820_device, is it really part of the public API?
>
> Let me check what can be done with it.
> Pavel
>

\
 
 \ /
  Last update: 2016-05-24 11:41    [W:0.191 / U:0.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site