| Message ID | 20230530173000.3060865-6-dave.stevenson@raspberrypi.com |
|---|---|
| State | New |
| Headers | show |
| Series | [01/21] media: i2c: imx258: Remove unused defines | expand |
Hi Dave On Tue, May 30, 2023 at 06:29:44PM +0100, Dave Stevenson wrote: > The device tree bindings define the relevant regulators for the > sensor, so update the driver to request the regulators and control > them at the appropriate times. > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > --- > drivers/media/i2c/imx258.c | 42 +++++++++++++++++++++++++++++++++++++- > 1 file changed, 41 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c > index b695fd987b71..30bae7388c3a 100644 > --- a/drivers/media/i2c/imx258.c > +++ b/drivers/media/i2c/imx258.c > @@ -7,6 +7,7 @@ > #include <linux/i2c.h> > #include <linux/module.h> > #include <linux/pm_runtime.h> > +#include <linux/regulator/consumer.h> > #include <media/v4l2-ctrls.h> > #include <media/v4l2-device.h> > #include <media/v4l2-fwnode.h> > @@ -507,6 +508,16 @@ static const char * const imx258_test_pattern_menu[] = { > "Pseudorandom Sequence (PN9)", > }; > > +/* regulator supplies */ > +static const char * const imx258_supply_name[] = { > + /* Supplies can be enabled in any order */ > + "vana", /* Analog (2.8V) supply */ > + "vdig", /* Digital Core (1.2V) supply */ > + "vif", /* IF (1.8V) supply */ > +}; > + > +#define IMX258_NUM_SUPPLIES ARRAY_SIZE(imx258_supply_name) > + > /* Configurations for supported link frequencies */ > #define IMX258_LINK_FREQ_634MHZ 633600000ULL > #define IMX258_LINK_FREQ_320MHZ 320000000ULL > @@ -614,6 +625,7 @@ struct imx258 { > bool streaming; > > struct clk *clk; > + struct regulator_bulk_data supplies[IMX258_NUM_SUPPLIES]; > }; > > static inline struct imx258 *to_imx258(struct v4l2_subdev *_sd) > @@ -999,9 +1011,19 @@ static int imx258_power_on(struct device *dev) > struct imx258 *imx258 = to_imx258(sd); > int ret; > > + ret = regulator_bulk_enable(IMX258_NUM_SUPPLIES, > + imx258->supplies); > + if (ret) { > + dev_err(dev, "%s: failed to enable regulators\n", > + __func__); > + return ret; > + } > + > ret = clk_prepare_enable(imx258->clk); > - if (ret) > + if (ret) { > dev_err(dev, "failed to enable clock\n"); > + regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies); > + } > > return ret; > } > @@ -1012,6 +1034,7 @@ static int imx258_power_off(struct device *dev) > struct imx258 *imx258 = to_imx258(sd); > > clk_disable_unprepare(imx258->clk); > + regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies); > > return 0; > } > @@ -1260,6 +1283,19 @@ static void imx258_free_controls(struct imx258 *imx258) > mutex_destroy(&imx258->mutex); > } > > +static int imx258_get_regulators(struct imx258 *imx258, > + struct i2c_client *client) > +{ > + unsigned int i; > + > + for (i = 0; i < IMX258_NUM_SUPPLIES; i++) > + imx258->supplies[i].supply = imx258_supply_name[i]; > + > + return devm_regulator_bulk_get(&client->dev, > + IMX258_NUM_SUPPLIES, > + imx258->supplies); nit: fits on 2 lines > +} > + > static int imx258_probe(struct i2c_client *client) > { > struct imx258 *imx258; > @@ -1270,6 +1306,10 @@ static int imx258_probe(struct i2c_client *client) > if (!imx258) > return -ENOMEM; > > + ret = imx258_get_regulators(imx258, client); > + if (ret) > + return ret; Is dev_err_probe() useful here ? > + > imx258->clk = devm_clk_get_optional(&client->dev, NULL); > if (IS_ERR(imx258->clk)) > return dev_err_probe(&client->dev, PTR_ERR(imx258->clk), > -- > 2.25.1 >
Hi Jacopo On Wed, 31 May 2023 at 16:11, Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote: > > Hi Dave > > On Tue, May 30, 2023 at 06:29:44PM +0100, Dave Stevenson wrote: > > The device tree bindings define the relevant regulators for the > > sensor, so update the driver to request the regulators and control > > them at the appropriate times. > > > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > --- > > drivers/media/i2c/imx258.c | 42 +++++++++++++++++++++++++++++++++++++- > > 1 file changed, 41 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c > > index b695fd987b71..30bae7388c3a 100644 > > --- a/drivers/media/i2c/imx258.c > > +++ b/drivers/media/i2c/imx258.c > > @@ -7,6 +7,7 @@ > > #include <linux/i2c.h> > > #include <linux/module.h> > > #include <linux/pm_runtime.h> > > +#include <linux/regulator/consumer.h> > > #include <media/v4l2-ctrls.h> > > #include <media/v4l2-device.h> > > #include <media/v4l2-fwnode.h> > > @@ -507,6 +508,16 @@ static const char * const imx258_test_pattern_menu[] = { > > "Pseudorandom Sequence (PN9)", > > }; > > > > +/* regulator supplies */ > > +static const char * const imx258_supply_name[] = { > > + /* Supplies can be enabled in any order */ > > + "vana", /* Analog (2.8V) supply */ > > + "vdig", /* Digital Core (1.2V) supply */ > > + "vif", /* IF (1.8V) supply */ > > +}; > > + > > +#define IMX258_NUM_SUPPLIES ARRAY_SIZE(imx258_supply_name) > > + > > /* Configurations for supported link frequencies */ > > #define IMX258_LINK_FREQ_634MHZ 633600000ULL > > #define IMX258_LINK_FREQ_320MHZ 320000000ULL > > @@ -614,6 +625,7 @@ struct imx258 { > > bool streaming; > > > > struct clk *clk; > > + struct regulator_bulk_data supplies[IMX258_NUM_SUPPLIES]; > > }; > > > > static inline struct imx258 *to_imx258(struct v4l2_subdev *_sd) > > @@ -999,9 +1011,19 @@ static int imx258_power_on(struct device *dev) > > struct imx258 *imx258 = to_imx258(sd); > > int ret; > > > > + ret = regulator_bulk_enable(IMX258_NUM_SUPPLIES, > > + imx258->supplies); > > + if (ret) { > > + dev_err(dev, "%s: failed to enable regulators\n", > > + __func__); > > + return ret; > > + } > > + > > ret = clk_prepare_enable(imx258->clk); > > - if (ret) > > + if (ret) { > > dev_err(dev, "failed to enable clock\n"); > > + regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies); > > + } > > > > return ret; > > } > > @@ -1012,6 +1034,7 @@ static int imx258_power_off(struct device *dev) > > struct imx258 *imx258 = to_imx258(sd); > > > > clk_disable_unprepare(imx258->clk); > > + regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies); > > > > return 0; > > } > > @@ -1260,6 +1283,19 @@ static void imx258_free_controls(struct imx258 *imx258) > > mutex_destroy(&imx258->mutex); > > } > > > > +static int imx258_get_regulators(struct imx258 *imx258, > > + struct i2c_client *client) > > +{ > > + unsigned int i; > > + > > + for (i = 0; i < IMX258_NUM_SUPPLIES; i++) > > + imx258->supplies[i].supply = imx258_supply_name[i]; > > + > > + return devm_regulator_bulk_get(&client->dev, > > + IMX258_NUM_SUPPLIES, > > + imx258->supplies); > > nit: fits on 2 lines Done > > +} > > + > > static int imx258_probe(struct i2c_client *client) > > { > > struct imx258 *imx258; > > @@ -1270,6 +1306,10 @@ static int imx258_probe(struct i2c_client *client) > > if (!imx258) > > return -ENOMEM; > > > > + ret = imx258_get_regulators(imx258, client); > > + if (ret) > > + return ret; > > Is dev_err_probe() useful here ? Sure, can do. Dave > > + > > imx258->clk = devm_clk_get_optional(&client->dev, NULL); > > if (IS_ERR(imx258->clk)) > > return dev_err_probe(&client->dev, PTR_ERR(imx258->clk), > > -- > > 2.25.1 > >
diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c index b695fd987b71..30bae7388c3a 100644 --- a/drivers/media/i2c/imx258.c +++ b/drivers/media/i2c/imx258.c @@ -7,6 +7,7 @@ #include <linux/i2c.h> #include <linux/module.h> #include <linux/pm_runtime.h> +#include <linux/regulator/consumer.h> #include <media/v4l2-ctrls.h> #include <media/v4l2-device.h> #include <media/v4l2-fwnode.h> @@ -507,6 +508,16 @@ static const char * const imx258_test_pattern_menu[] = { "Pseudorandom Sequence (PN9)", }; +/* regulator supplies */ +static const char * const imx258_supply_name[] = { + /* Supplies can be enabled in any order */ + "vana", /* Analog (2.8V) supply */ + "vdig", /* Digital Core (1.2V) supply */ + "vif", /* IF (1.8V) supply */ +}; + +#define IMX258_NUM_SUPPLIES ARRAY_SIZE(imx258_supply_name) + /* Configurations for supported link frequencies */ #define IMX258_LINK_FREQ_634MHZ 633600000ULL #define IMX258_LINK_FREQ_320MHZ 320000000ULL @@ -614,6 +625,7 @@ struct imx258 { bool streaming; struct clk *clk; + struct regulator_bulk_data supplies[IMX258_NUM_SUPPLIES]; }; static inline struct imx258 *to_imx258(struct v4l2_subdev *_sd) @@ -999,9 +1011,19 @@ static int imx258_power_on(struct device *dev) struct imx258 *imx258 = to_imx258(sd); int ret; + ret = regulator_bulk_enable(IMX258_NUM_SUPPLIES, + imx258->supplies); + if (ret) { + dev_err(dev, "%s: failed to enable regulators\n", + __func__); + return ret; + } + ret = clk_prepare_enable(imx258->clk); - if (ret) + if (ret) { dev_err(dev, "failed to enable clock\n"); + regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies); + } return ret; } @@ -1012,6 +1034,7 @@ static int imx258_power_off(struct device *dev) struct imx258 *imx258 = to_imx258(sd); clk_disable_unprepare(imx258->clk); + regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies); return 0; } @@ -1260,6 +1283,19 @@ static void imx258_free_controls(struct imx258 *imx258) mutex_destroy(&imx258->mutex); } +static int imx258_get_regulators(struct imx258 *imx258, + struct i2c_client *client) +{ + unsigned int i; + + for (i = 0; i < IMX258_NUM_SUPPLIES; i++) + imx258->supplies[i].supply = imx258_supply_name[i]; + + return devm_regulator_bulk_get(&client->dev, + IMX258_NUM_SUPPLIES, + imx258->supplies); +} + static int imx258_probe(struct i2c_client *client) { struct imx258 *imx258; @@ -1270,6 +1306,10 @@ static int imx258_probe(struct i2c_client *client) if (!imx258) return -ENOMEM; + ret = imx258_get_regulators(imx258, client); + if (ret) + return ret; + imx258->clk = devm_clk_get_optional(&client->dev, NULL); if (IS_ERR(imx258->clk)) return dev_err_probe(&client->dev, PTR_ERR(imx258->clk),
The device tree bindings define the relevant regulators for the sensor, so update the driver to request the regulators and control them at the appropriate times. Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> --- drivers/media/i2c/imx258.c | 42 +++++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-)