From: Jonathan Cameron <Jonathan.Cameron@Huawei.com> To: Billy Tsai <billy_tsai@aspeedtech.com> Cc: <jic23@kernel.org>, <lars@metafoo.de>, <pmeerw@pmeerw.net>, <robh+dt@kernel.org>, <joel@jms.id.au>, <andrew@aj.id.au>, <p.zabel@pengutronix.de>, <linux-iio@vger.kernel.org>, <devicetree@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>, <linux-aspeed@lists.ozlabs.org>, <linux-kernel@vger.kernel.org>, <BMC-SW@aspeedtech.com> Subject: Re: [v2 3/8] iio: adc: aspeed: completes the bitfield declare. Date: Fri, 23 Jul 2021 15:55:30 +0100 [thread overview] Message-ID: <20210723155530.00000d1c@Huawei.com> (raw) In-Reply-To: <20210723081621.29477-4-billy_tsai@aspeedtech.com> On Fri, 23 Jul 2021 16:16:16 +0800 Billy Tsai <billy_tsai@aspeedtech.com> wrote: > This patch completes the declare of adc register bitfields and uses the > same prefix ASPEED_ADC_* for these bitfields. > > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com> Hi Billy See inline, Thanks, Jonathan > --- > drivers/iio/adc/aspeed_adc.c | 40 ++++++++++++++++++++++++------------ > 1 file changed, 27 insertions(+), 13 deletions(-) > > diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c > index 19efaa41bc34..99466a5924c7 100644 > --- a/drivers/iio/adc/aspeed_adc.c > +++ b/drivers/iio/adc/aspeed_adc.c > @@ -16,6 +16,7 @@ > #include <linux/reset.h> > #include <linux/spinlock.h> > #include <linux/types.h> > +#include <linux/bitfield.h> > > #include <linux/iio/iio.h> > #include <linux/iio/driver.h> > @@ -28,15 +29,28 @@ > #define ASPEED_REG_INTERRUPT_CONTROL 0x04 > #define ASPEED_REG_VGA_DETECT_CONTROL 0x08 > #define ASPEED_REG_CLOCK_CONTROL 0x0C > -#define ASPEED_REG_MAX 0xC0 > - > -#define ASPEED_OPERATION_MODE_POWER_DOWN (0x0 << 1) > -#define ASPEED_OPERATION_MODE_STANDBY (0x1 << 1) > -#define ASPEED_OPERATION_MODE_NORMAL (0x7 << 1) > - > -#define ASPEED_ENGINE_ENABLE BIT(0) > - > -#define ASPEED_ADC_CTRL_INIT_RDY BIT(8) > +#define ASPEED_REG_COMPENSATION_TRIM 0xC4 > +#define ASPEED_REG_MAX 0xCC > + > +#define ASPEED_ADC_ENGINE_ENABLE BIT(0) > +#define ASPEED_ADC_OPERATION_MODE GENMASK(3, 1) > +#define ASPEED_ADC_OPERATION_MODE_POWER_DOWN FIELD_PREP(ASPEED_ADC_OPERATION_MODE, 0) It's more common to have the FIELD_PREP at the location where it is used and just have defines here to be to the value of the field. Perhaps also consider some abbreviations as I think we can safely make them here without losing any meaning, given context. ASPEED_ADC_OP_MODE ASPEED_ADC_OP_MODE_PWR_DWN ASPEED_ADC_OP_MODE_STANDBY ASPEED_ADC_OP_MODE_NORMAL etc. > +#define ASPEED_ADC_OPERATION_MODE_STANDBY FIELD_PREP(ASPEED_ADC_OPERATION_MODE, 1) > +#define ASPEED_ADC_OPERATION_MODE_NORMAL FIELD_PREP(ASPEED_ADC_OPERATION_MODE, 7) > +#define ASPEED_ADC_CTRL_COMPENSATION BIT(4) > +#define ASPEED_ADC_AUTO_COMPENSATION BIT(5) > +#define ASPEED_ADC_REF_VOLTAGE GENMASK(7, 6) > +#define ASPEED_ADC_REF_VOLTAGE_2500mV FIELD_PREP(ASPEED_ADC_REF_VOLTAGE, 0) > +#define ASPEED_ADC_REF_VOLTAGE_1200mV FIELD_PREP(ASPEED_ADC_REF_VOLTAGE, 1) > +#define ASPEED_ADC_REF_VOLTAGE_EXT_HIGH FIELD_PREP(ASPEED_ADC_REF_VOLTAGE, 2) > +#define ASPEED_ADC_REF_VOLTAGE_EXT_LOW FIELD_PREP(ASPEED_ADC_REF_VOLTAGE, 3) > +#define ASPEED_ADC_CTRL_INIT_RDY BIT(8) > +#define ASPEED_ADC_CH7_MODE BIT(12) > +#define ASPEED_ADC_CH7_NORMAL FIELD_PREP(ASPEED_ADC_CH7_MODE, 0) > +#define ASPEED_ADC_CH7_BATTERY FIELD_PREP(ASPEED_ADC_CH7_MODE, 1) > +#define ASPEED_ADC_BATTERY_SENSING_ENABLE BIT(13) > +#define ASPEED_ADC_CTRL_CHANNEL GENMASK(31, 16) > +#define ASPEED_ADC_CTRL_CHANNEL_ENABLE(ch) FIELD_PREP(ASPEED_ADC_CTRL_CHANNEL, BIT(ch)) > > #define ASPEED_ADC_INIT_POLLING_TIME 500 > #define ASPEED_ADC_INIT_TIMEOUT 500000 > @@ -226,7 +240,7 @@ static int aspeed_adc_probe(struct platform_device *pdev) > > if (model_data->wait_init_sequence) { > /* Enable engine in normal mode. */ > - writel(ASPEED_OPERATION_MODE_NORMAL | ASPEED_ENGINE_ENABLE, > + writel(ASPEED_ADC_OPERATION_MODE_NORMAL | ASPEED_ADC_ENGINE_ENABLE, > data->base + ASPEED_REG_ENGINE_CONTROL); > > /* Wait for initial sequence complete. */ > @@ -246,7 +260,7 @@ static int aspeed_adc_probe(struct platform_device *pdev) > goto clk_enable_error; > > adc_engine_control_reg_val = GENMASK(31, 16) | > - ASPEED_OPERATION_MODE_NORMAL | ASPEED_ENGINE_ENABLE; > + ASPEED_ADC_OPERATION_MODE_NORMAL | ASPEED_ADC_ENGINE_ENABLE; > writel(adc_engine_control_reg_val, > data->base + ASPEED_REG_ENGINE_CONTROL); > > @@ -264,7 +278,7 @@ static int aspeed_adc_probe(struct platform_device *pdev) > return 0; > > iio_register_error: > - writel(ASPEED_OPERATION_MODE_POWER_DOWN, > + writel(ASPEED_ADC_OPERATION_MODE_POWER_DOWN, > data->base + ASPEED_REG_ENGINE_CONTROL); > clk_disable_unprepare(data->clk_scaler->clk); > clk_enable_error: > @@ -283,7 +297,7 @@ static int aspeed_adc_remove(struct platform_device *pdev) > struct aspeed_adc_data *data = iio_priv(indio_dev); > > iio_device_unregister(indio_dev); > - writel(ASPEED_OPERATION_MODE_POWER_DOWN, > + writel(ASPEED_ADC_OPERATION_MODE_POWER_DOWN, > data->base + ASPEED_REG_ENGINE_CONTROL); > clk_disable_unprepare(data->clk_scaler->clk); > reset_control_assert(data->rst);
WARNING: multiple messages have this Message-ID (diff)
From: Jonathan Cameron <Jonathan.Cameron@Huawei.com> To: Billy Tsai <billy_tsai@aspeedtech.com> Cc: <jic23@kernel.org>, <lars@metafoo.de>, <pmeerw@pmeerw.net>, <robh+dt@kernel.org>, <joel@jms.id.au>, <andrew@aj.id.au>, <p.zabel@pengutronix.de>, <linux-iio@vger.kernel.org>, <devicetree@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>, <linux-aspeed@lists.ozlabs.org>, <linux-kernel@vger.kernel.org>, <BMC-SW@aspeedtech.com> Subject: Re: [v2 3/8] iio: adc: aspeed: completes the bitfield declare. Date: Fri, 23 Jul 2021 15:55:30 +0100 [thread overview] Message-ID: <20210723155530.00000d1c@Huawei.com> (raw) In-Reply-To: <20210723081621.29477-4-billy_tsai@aspeedtech.com> On Fri, 23 Jul 2021 16:16:16 +0800 Billy Tsai <billy_tsai@aspeedtech.com> wrote: > This patch completes the declare of adc register bitfields and uses the > same prefix ASPEED_ADC_* for these bitfields. > > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com> Hi Billy See inline, Thanks, Jonathan > --- > drivers/iio/adc/aspeed_adc.c | 40 ++++++++++++++++++++++++------------ > 1 file changed, 27 insertions(+), 13 deletions(-) > > diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c > index 19efaa41bc34..99466a5924c7 100644 > --- a/drivers/iio/adc/aspeed_adc.c > +++ b/drivers/iio/adc/aspeed_adc.c > @@ -16,6 +16,7 @@ > #include <linux/reset.h> > #include <linux/spinlock.h> > #include <linux/types.h> > +#include <linux/bitfield.h> > > #include <linux/iio/iio.h> > #include <linux/iio/driver.h> > @@ -28,15 +29,28 @@ > #define ASPEED_REG_INTERRUPT_CONTROL 0x04 > #define ASPEED_REG_VGA_DETECT_CONTROL 0x08 > #define ASPEED_REG_CLOCK_CONTROL 0x0C > -#define ASPEED_REG_MAX 0xC0 > - > -#define ASPEED_OPERATION_MODE_POWER_DOWN (0x0 << 1) > -#define ASPEED_OPERATION_MODE_STANDBY (0x1 << 1) > -#define ASPEED_OPERATION_MODE_NORMAL (0x7 << 1) > - > -#define ASPEED_ENGINE_ENABLE BIT(0) > - > -#define ASPEED_ADC_CTRL_INIT_RDY BIT(8) > +#define ASPEED_REG_COMPENSATION_TRIM 0xC4 > +#define ASPEED_REG_MAX 0xCC > + > +#define ASPEED_ADC_ENGINE_ENABLE BIT(0) > +#define ASPEED_ADC_OPERATION_MODE GENMASK(3, 1) > +#define ASPEED_ADC_OPERATION_MODE_POWER_DOWN FIELD_PREP(ASPEED_ADC_OPERATION_MODE, 0) It's more common to have the FIELD_PREP at the location where it is used and just have defines here to be to the value of the field. Perhaps also consider some abbreviations as I think we can safely make them here without losing any meaning, given context. ASPEED_ADC_OP_MODE ASPEED_ADC_OP_MODE_PWR_DWN ASPEED_ADC_OP_MODE_STANDBY ASPEED_ADC_OP_MODE_NORMAL etc. > +#define ASPEED_ADC_OPERATION_MODE_STANDBY FIELD_PREP(ASPEED_ADC_OPERATION_MODE, 1) > +#define ASPEED_ADC_OPERATION_MODE_NORMAL FIELD_PREP(ASPEED_ADC_OPERATION_MODE, 7) > +#define ASPEED_ADC_CTRL_COMPENSATION BIT(4) > +#define ASPEED_ADC_AUTO_COMPENSATION BIT(5) > +#define ASPEED_ADC_REF_VOLTAGE GENMASK(7, 6) > +#define ASPEED_ADC_REF_VOLTAGE_2500mV FIELD_PREP(ASPEED_ADC_REF_VOLTAGE, 0) > +#define ASPEED_ADC_REF_VOLTAGE_1200mV FIELD_PREP(ASPEED_ADC_REF_VOLTAGE, 1) > +#define ASPEED_ADC_REF_VOLTAGE_EXT_HIGH FIELD_PREP(ASPEED_ADC_REF_VOLTAGE, 2) > +#define ASPEED_ADC_REF_VOLTAGE_EXT_LOW FIELD_PREP(ASPEED_ADC_REF_VOLTAGE, 3) > +#define ASPEED_ADC_CTRL_INIT_RDY BIT(8) > +#define ASPEED_ADC_CH7_MODE BIT(12) > +#define ASPEED_ADC_CH7_NORMAL FIELD_PREP(ASPEED_ADC_CH7_MODE, 0) > +#define ASPEED_ADC_CH7_BATTERY FIELD_PREP(ASPEED_ADC_CH7_MODE, 1) > +#define ASPEED_ADC_BATTERY_SENSING_ENABLE BIT(13) > +#define ASPEED_ADC_CTRL_CHANNEL GENMASK(31, 16) > +#define ASPEED_ADC_CTRL_CHANNEL_ENABLE(ch) FIELD_PREP(ASPEED_ADC_CTRL_CHANNEL, BIT(ch)) > > #define ASPEED_ADC_INIT_POLLING_TIME 500 > #define ASPEED_ADC_INIT_TIMEOUT 500000 > @@ -226,7 +240,7 @@ static int aspeed_adc_probe(struct platform_device *pdev) > > if (model_data->wait_init_sequence) { > /* Enable engine in normal mode. */ > - writel(ASPEED_OPERATION_MODE_NORMAL | ASPEED_ENGINE_ENABLE, > + writel(ASPEED_ADC_OPERATION_MODE_NORMAL | ASPEED_ADC_ENGINE_ENABLE, > data->base + ASPEED_REG_ENGINE_CONTROL); > > /* Wait for initial sequence complete. */ > @@ -246,7 +260,7 @@ static int aspeed_adc_probe(struct platform_device *pdev) > goto clk_enable_error; > > adc_engine_control_reg_val = GENMASK(31, 16) | > - ASPEED_OPERATION_MODE_NORMAL | ASPEED_ENGINE_ENABLE; > + ASPEED_ADC_OPERATION_MODE_NORMAL | ASPEED_ADC_ENGINE_ENABLE; > writel(adc_engine_control_reg_val, > data->base + ASPEED_REG_ENGINE_CONTROL); > > @@ -264,7 +278,7 @@ static int aspeed_adc_probe(struct platform_device *pdev) > return 0; > > iio_register_error: > - writel(ASPEED_OPERATION_MODE_POWER_DOWN, > + writel(ASPEED_ADC_OPERATION_MODE_POWER_DOWN, > data->base + ASPEED_REG_ENGINE_CONTROL); > clk_disable_unprepare(data->clk_scaler->clk); > clk_enable_error: > @@ -283,7 +297,7 @@ static int aspeed_adc_remove(struct platform_device *pdev) > struct aspeed_adc_data *data = iio_priv(indio_dev); > > iio_device_unregister(indio_dev); > - writel(ASPEED_OPERATION_MODE_POWER_DOWN, > + writel(ASPEED_ADC_OPERATION_MODE_POWER_DOWN, > data->base + ASPEED_REG_ENGINE_CONTROL); > clk_disable_unprepare(data->clk_scaler->clk); > reset_control_assert(data->rst); _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-07-23 14:56 UTC|newest] Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-07-23 8:16 [v2 0/8] Add support for ast2600 ADC Billy Tsai 2021-07-23 8:16 ` Billy Tsai 2021-07-23 8:16 ` [v2 1/8] dt-bindings: iio: adc: rename the aspeed adc yaml Billy Tsai 2021-07-23 8:16 ` Billy Tsai 2021-07-23 14:44 ` Jonathan Cameron 2021-07-23 14:44 ` Jonathan Cameron 2021-07-26 6:53 ` Billy Tsai 2021-07-26 6:53 ` Billy Tsai 2021-07-29 20:31 ` Rob Herring 2021-07-29 20:31 ` Rob Herring 2021-07-31 17:27 ` Jonathan Cameron 2021-07-31 17:27 ` Jonathan Cameron 2021-07-23 8:16 ` [v2 2/8] dt-bindings: iio: adc: Binding ast2600 adc Billy Tsai 2021-07-23 8:16 ` Billy Tsai 2021-07-23 14:51 ` Jonathan Cameron 2021-07-23 14:51 ` Jonathan Cameron 2021-07-26 7:21 ` Billy Tsai 2021-07-26 7:21 ` Billy Tsai 2021-07-23 8:16 ` [v2 3/8] iio: adc: aspeed: completes the bitfield declare Billy Tsai 2021-07-23 8:16 ` Billy Tsai 2021-07-23 14:55 ` Jonathan Cameron [this message] 2021-07-23 14:55 ` Jonathan Cameron 2021-07-23 8:16 ` [v2 4/8] iio: adc: aspeed: Allow driver to support ast2600 Billy Tsai 2021-07-23 8:16 ` Billy Tsai 2021-07-23 15:29 ` Jonathan Cameron 2021-07-23 15:29 ` Jonathan Cameron 2021-07-23 8:16 ` [v2 5/8] iio: adc: aspeed: Add func to set sampling rate Billy Tsai 2021-07-23 8:16 ` Billy Tsai 2021-07-23 15:37 ` Jonathan Cameron 2021-07-23 15:37 ` Jonathan Cameron 2021-07-23 8:16 ` [v2 6/8] iio: adc: aspeed: Add compensation phase Billy Tsai 2021-07-23 8:16 ` Billy Tsai 2021-07-23 15:44 ` Jonathan Cameron 2021-07-23 15:44 ` Jonathan Cameron 2021-07-23 8:16 ` [v2 7/8] iio: adc: aspeed: Fix the calculate error of clock Billy Tsai 2021-07-23 8:16 ` Billy Tsai 2021-07-23 8:16 ` [v2 8/8] iio: adc: aspeed: Support battery sensing Billy Tsai 2021-07-23 8:16 ` Billy Tsai 2021-07-23 15:56 ` Jonathan Cameron 2021-07-23 15:56 ` Jonathan Cameron 2021-07-26 6:54 ` Billy Tsai 2021-07-26 6:54 ` Billy Tsai
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210723155530.00000d1c@Huawei.com \ --to=jonathan.cameron@huawei.com \ --cc=BMC-SW@aspeedtech.com \ --cc=andrew@aj.id.au \ --cc=billy_tsai@aspeedtech.com \ --cc=devicetree@vger.kernel.org \ --cc=jic23@kernel.org \ --cc=joel@jms.id.au \ --cc=lars@metafoo.de \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-aspeed@lists.ozlabs.org \ --cc=linux-iio@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=p.zabel@pengutronix.de \ --cc=pmeerw@pmeerw.net \ --cc=robh+dt@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.