Re: [PATCH v2 2/4] drm/bridge: adv7511: Initialize regulators

[Archit Taneja <architt@codeaurora.org>]

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@lists.freedesktop.org
>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Signed-off-by: Archit Taneja <architt@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@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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel