| Message ID | 20240717212535.8348-5-alisa.roman@analog.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | iio: adc: adc7192: Improvements | expand |
On Thu, 2024-07-18 at 00:25 +0300, Alisa-Dariana Roman wrote: > Internal clock of AD719X devices can be made available on MCLK2 pin. Add > clock provider to support this functionality when clock cells property > is present. > > Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com> > --- minor thing below you may consider if a re-spin is needed... Reviewed-by: Nuno Sa <nuno.sa@analog.com> > drivers/iio/adc/ad7192.c | 92 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 92 insertions(+) > > diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c > index 042319f0c641..3f803b1eefcc 100644 > --- a/drivers/iio/adc/ad7192.c > +++ b/drivers/iio/adc/ad7192.c > @@ -8,6 +8,7 @@ > #include <linux/interrupt.h> > #include <linux/bitfield.h> > #include <linux/clk.h> > +#include <linux/clk-provider.h> > #include <linux/device.h> > #include <linux/kernel.h> > #include <linux/slab.h> > @@ -201,6 +202,7 @@ struct ad7192_chip_info { > struct ad7192_state { > const struct ad7192_chip_info *chip_info; > struct clk *mclk; > + struct clk_hw int_clk_hw; > u16 int_vref_mv; > u32 aincom_mv; > u32 fclk; > @@ -406,6 +408,91 @@ static const char *const ad7192_clock_names[] = { > "mclk" > }; > > +static struct ad7192_state *clk_hw_to_ad7192(struct clk_hw *hw) > +{ > + return container_of(hw, struct ad7192_state, int_clk_hw); > +} > + > +static unsigned long ad7192_clk_recalc_rate(struct clk_hw *hw, > + unsigned long parent_rate) > +{ > + return AD7192_INT_FREQ_MHZ; > +} > + > +static int ad7192_clk_output_is_enabled(struct clk_hw *hw) > +{ > + struct ad7192_state *st = clk_hw_to_ad7192(hw); > + > + return st->clock_sel == AD7192_CLK_INT_CO; > +} > + > +static int ad7192_clk_prepare(struct clk_hw *hw) > +{ > + struct ad7192_state *st = clk_hw_to_ad7192(hw); > + int ret; > + > + st->mode &= ~AD7192_MODE_CLKSRC_MASK; > + st->mode |= AD7192_CLK_INT_CO; > + > + ret = ad_sd_write_reg(&st->sd, AD7192_REG_MODE, 3, st->mode); > + if (ret) > + return ret; > + > + st->clock_sel = AD7192_CLK_INT_CO; > + > + return 0; > +} > + > +static void ad7192_clk_unprepare(struct clk_hw *hw) > +{ > + struct ad7192_state *st = clk_hw_to_ad7192(hw); > + int ret; > + > + st->mode &= ~AD7192_MODE_CLKSRC_MASK; > + st->mode |= AD7192_CLK_INT; > + > + ret = ad_sd_write_reg(&st->sd, AD7192_REG_MODE, 3, st->mode); > + if (ret) > + return; > + > + st->clock_sel = AD7192_CLK_INT; > +} > + > +static const struct clk_ops ad7192_int_clk_ops = { > + .recalc_rate = ad7192_clk_recalc_rate, > + .is_enabled = ad7192_clk_output_is_enabled, > + .prepare = ad7192_clk_prepare, > + .unprepare = ad7192_clk_unprepare, > +}; > + > +static int ad7192_register_clk_provider(struct ad7192_state *st) > +{ > + struct device *dev = &st->sd.spi->dev; > + struct clk_init_data init = {}; > + int ret; > + > + if (!device_property_present(dev, "#clock-cells")) > + return 0; > + > + if (!IS_ENABLED(CONFIG_COMMON_CLK)) > + return 0; > nit: This could be the first test to do. No point in calling device_property_present() if CONFIG_COMMON_CLK is disabled. FWIW, the compiler should be smart enough to sort things out but it would still be better (for readability) to have this first. - Nuno Sá
On Thu, 18 Jul 2024 16:11:29 +0200 Nuno Sá <noname.nuno@gmail.com> wrote: > On Thu, 2024-07-18 at 00:25 +0300, Alisa-Dariana Roman wrote: > > Internal clock of AD719X devices can be made available on MCLK2 pin. Add > > clock provider to support this functionality when clock cells property > > is present. > > > > Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com> > > --- > > minor thing below you may consider if a re-spin is needed... > > Reviewed-by: Nuno Sa <nuno.sa@analog.com> > > +static int ad7192_register_clk_provider(struct ad7192_state *st) > > +{ > > + struct device *dev = &st->sd.spi->dev; > > + struct clk_init_data init = {}; > > + int ret; > > + > > + if (!device_property_present(dev, "#clock-cells")) > > + return 0; > > + > > + if (!IS_ENABLED(CONFIG_COMMON_CLK)) > > + return 0; > > > > nit: This could be the first test to do. No point in calling > device_property_present() if CONFIG_COMMON_CLK is disabled. FWIW, the compiler should > be smart enough to sort things out but it would still be better (for readability) to > have this first. > Tweaked whilst applying. Compiler probably can't figure it out as won't have enough visibility of the implementation of device_property_present() to know it doesn't have side effects deep in one of the indirect function calls that can generate. Jonathan > - Nuno Sá > >
diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c index 042319f0c641..3f803b1eefcc 100644 --- a/drivers/iio/adc/ad7192.c +++ b/drivers/iio/adc/ad7192.c @@ -8,6 +8,7 @@ #include <linux/interrupt.h> #include <linux/bitfield.h> #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/device.h> #include <linux/kernel.h> #include <linux/slab.h> @@ -201,6 +202,7 @@ struct ad7192_chip_info { struct ad7192_state { const struct ad7192_chip_info *chip_info; struct clk *mclk; + struct clk_hw int_clk_hw; u16 int_vref_mv; u32 aincom_mv; u32 fclk; @@ -406,6 +408,91 @@ static const char *const ad7192_clock_names[] = { "mclk" }; +static struct ad7192_state *clk_hw_to_ad7192(struct clk_hw *hw) +{ + return container_of(hw, struct ad7192_state, int_clk_hw); +} + +static unsigned long ad7192_clk_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + return AD7192_INT_FREQ_MHZ; +} + +static int ad7192_clk_output_is_enabled(struct clk_hw *hw) +{ + struct ad7192_state *st = clk_hw_to_ad7192(hw); + + return st->clock_sel == AD7192_CLK_INT_CO; +} + +static int ad7192_clk_prepare(struct clk_hw *hw) +{ + struct ad7192_state *st = clk_hw_to_ad7192(hw); + int ret; + + st->mode &= ~AD7192_MODE_CLKSRC_MASK; + st->mode |= AD7192_CLK_INT_CO; + + ret = ad_sd_write_reg(&st->sd, AD7192_REG_MODE, 3, st->mode); + if (ret) + return ret; + + st->clock_sel = AD7192_CLK_INT_CO; + + return 0; +} + +static void ad7192_clk_unprepare(struct clk_hw *hw) +{ + struct ad7192_state *st = clk_hw_to_ad7192(hw); + int ret; + + st->mode &= ~AD7192_MODE_CLKSRC_MASK; + st->mode |= AD7192_CLK_INT; + + ret = ad_sd_write_reg(&st->sd, AD7192_REG_MODE, 3, st->mode); + if (ret) + return; + + st->clock_sel = AD7192_CLK_INT; +} + +static const struct clk_ops ad7192_int_clk_ops = { + .recalc_rate = ad7192_clk_recalc_rate, + .is_enabled = ad7192_clk_output_is_enabled, + .prepare = ad7192_clk_prepare, + .unprepare = ad7192_clk_unprepare, +}; + +static int ad7192_register_clk_provider(struct ad7192_state *st) +{ + struct device *dev = &st->sd.spi->dev; + struct clk_init_data init = {}; + int ret; + + if (!device_property_present(dev, "#clock-cells")) + return 0; + + if (!IS_ENABLED(CONFIG_COMMON_CLK)) + return 0; + + init.name = devm_kasprintf(dev, GFP_KERNEL, "%s-clk", + fwnode_get_name(dev_fwnode(dev))); + if (!init.name) + return -ENOMEM; + + init.ops = &ad7192_int_clk_ops; + + st->int_clk_hw.init = &init; + ret = devm_clk_hw_register(dev, &st->int_clk_hw); + if (ret) + return ret; + + return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, + &st->int_clk_hw); +} + static int ad7192_clock_setup(struct ad7192_state *st) { struct device *dev = &st->sd.spi->dev; @@ -446,6 +533,11 @@ static int ad7192_clock_setup(struct ad7192_state *st) if (ret < 0) { st->clock_sel = AD7192_CLK_INT; st->fclk = AD7192_INT_FREQ_MHZ; + + ret = ad7192_register_clk_provider(st); + if (ret) + return dev_err_probe(dev, ret, + "Failed to register clock provider\n"); return 0; }
Internal clock of AD719X devices can be made available on MCLK2 pin. Add clock provider to support this functionality when clock cells property is present. Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com> --- drivers/iio/adc/ad7192.c | 92 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+)