On 9/25/2016 11:05 PM, Laurent Pinchart wrote:
> Hi Archit,
>
> Thank you for the patch.
>
> On Friday 23 Sep 2016 14:50:28 Archit Taneja wrote:
>> Maintain a table of regulator names expect by ADV7511 and ADV7533.
>> Use regulator_bulk_* api to configure these.
>>
>> Initialize and enable the regulators during probe itself. Controlling
>> these dynamically is left for later.
>>
>> Cc: dri-devel at lists.freedesktop.org
>> Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>> Signed-off-by: Archit Taneja <architt at codeaurora.org>
>> ---
>> v2:
>> - Use regulator_bulk_* API to configure regulators as suggested by Laurent.
>> - Set up regulators for ADV7511 too.
>>
>>   drivers/gpu/drm/bridge/adv7511/adv7511.h     |  4 ++
>>   drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 85 ++++++++++++++++++++++---
>>   2 files changed, 80 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
>> b/drivers/gpu/drm/bridge/adv7511/adv7511.h index 161c923..83ebdaa 100644
>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
>> @@ -12,6 +12,7 @@
>>   #include <linux/hdmi.h>
>>   #include <linux/i2c.h>
>>   #include <linux/regmap.h>
>> +#include <linux/regulator/consumer.h>
>>
>>   #include <drm/drm_crtc_helper.h>
>>   #include <drm/drm_mipi_dsi.h>
>> @@ -327,6 +328,9 @@ struct adv7511 {
>>
>>      struct gpio_desc *gpio_pd;
>>
>> +    struct regulator_bulk_data *supplies;
>> +    int num_supplies;
>
> num_supplies is always positive, you can make it an unsigned int.
>
>> +
>>      /* ADV7533 DSI RX related params */
>>      struct device_node *host_node;
>>      struct mipi_dsi_device *dsi;
>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 8ed3906..f7e79ed
>> 100644
>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> @@ -839,6 +839,57 @@ static struct drm_bridge_funcs adv7511_bridge_funcs = {
>> * Probe & remove
>>    */
>>
>> +static const char * const adv7511_supply_names[] = {
>> +    "avdd",
>> +    "dvdd",
>> +    "pvdd",
>> +    "v3p3",
>> +    "bgvdd",
>> +};
>> +
>> +static const char * const adv7533_supply_names[] = {
>> +    "avdd",
>> +    "dvdd",
>> +    "pvdd",
>> +    "a2vdd",
>> +    "v3p3",
>> +    "v1p2",
>> +};
>> +
>> +static int adv7511_init_regulators(struct adv7511 *adv)
>> +{
>> +    struct device *dev = &adv->i2c_main->dev;
>> +    const char * const *supply_names;
>> +    int i, ret;
>
> i only takes positive values, you can make it an unsigned int.
>
> Apart from that,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

I'll fix these. Thanks for the review.

Archit

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Reply via email to