The Preset Register (PR), Flag Register (FLAG), and Filter Clock
Prescaler (PSC) have common usage patterns. Wrap up such usage into
dedicated functions to improve code clarity.
Signed-off-by: William Breathitt Gray <william.gray@linaro.org>
---
Changes in v2:
- Utilize ioread8_rep() and iowrite8_rep() to read and write counter
data
drivers/counter/104-quad-8.c | 87 +++++++++++++++---------------------
1 file changed, 37 insertions(+), 50 deletions(-)
diff --git a/drivers/counter/104-quad-8.c b/drivers/counter/104-quad-8.c
index fe0887e6185d..02c5499378b6 100644
--- a/drivers/counter/104-quad-8.c
+++ b/drivers/counter/104-quad-8.c
@@ -231,52 +231,56 @@ static int quad8_count_read(struct counter_device *counter,
struct quad8 *const priv = counter_priv(counter);
struct channel_reg __iomem *const chan = priv->reg->channel + count->id;
unsigned long irqflags;
- int i;
*val = 0;
spin_lock_irqsave(&priv->lock, irqflags);
iowrite8(SELECT_RLD | RESET_BP | TRANSFER_CNTR_TO_OL, &chan->control);
-
- for (i = 0; i < 3; i++)
- *val |= (unsigned long)ioread8(&chan->data) << (8 * i);
+ ioread8_rep(&chan->data, val, 3);
spin_unlock_irqrestore(&priv->lock, irqflags);
return 0;
}
+static void quad8_preset_register_set(struct quad8 *const priv, const size_t id,
+ const unsigned long preset)
+{
+ struct channel_reg __iomem *const chan = priv->reg->channel + id;
+
+ iowrite8(SELECT_RLD | RESET_BP, &chan->control);
+ iowrite8_rep(&chan->data, &preset, 3);
+}
+
+static void quad8_flag_register_reset(struct quad8 *const priv, const size_t id)
+{
+ struct channel_reg __iomem *const chan = priv->reg->channel + id;
+
+ iowrite8(SELECT_RLD | RESET_BT_CT_CPT_S_IDX, &chan->control);
+ iowrite8(SELECT_RLD | RESET_E, &chan->control);
+}
+
static int quad8_count_write(struct counter_device *counter,
struct counter_count *count, u64 val)
{
struct quad8 *const priv = counter_priv(counter);
struct channel_reg __iomem *const chan = priv->reg->channel + count->id;
unsigned long irqflags;
- int i;
if (val > LS7267_CNTR_MAX)
return -ERANGE;
spin_lock_irqsave(&priv->lock, irqflags);
- iowrite8(SELECT_RLD | RESET_BP, &chan->control);
-
/* Counter can only be set via Preset Register */
- for (i = 0; i < 3; i++)
- iowrite8(val >> (8 * i), &chan->data);
-
+ quad8_preset_register_set(priv, count->id, val);
iowrite8(SELECT_RLD | TRANSFER_PR_TO_CNTR, &chan->control);
- iowrite8(SELECT_RLD | RESET_BP, &chan->control);
+ quad8_flag_register_reset(priv, count->id);
/* Set Preset Register back to original value */
- val = priv->preset[count->id];
- for (i = 0; i < 3; i++)
- iowrite8(val >> (8 * i), &chan->data);
-
- iowrite8(SELECT_RLD | RESET_BT_CT_CPT_S_IDX, &chan->control);
- iowrite8(SELECT_RLD | RESET_E, &chan->control);
+ quad8_preset_register_set(priv, count->id, priv->preset[count->id]);
spin_unlock_irqrestore(&priv->lock, irqflags);
@@ -770,21 +774,6 @@ static int quad8_count_preset_read(struct counter_device *counter,
return 0;
}
-static void quad8_preset_register_set(struct quad8 *const priv, const int id,
- const unsigned int preset)
-{
- struct channel_reg __iomem *const chan = priv->reg->channel + id;
- int i;
-
- priv->preset[id] = preset;
-
- iowrite8(SELECT_RLD | RESET_BP, &chan->control);
-
- /* Set Preset Register */
- for (i = 0; i < 3; i++)
- iowrite8(preset >> (8 * i), &chan->data);
-}
-
static int quad8_count_preset_write(struct counter_device *counter,
struct counter_count *count, u64 preset)
{
@@ -796,6 +785,7 @@ static int quad8_count_preset_write(struct counter_device *counter,
spin_lock_irqsave(&priv->lock, irqflags);
+ priv->preset[count->id] = preset;
quad8_preset_register_set(priv, count->id, preset);
spin_unlock_irqrestore(&priv->lock, irqflags);
@@ -842,6 +832,7 @@ static int quad8_count_ceiling_write(struct counter_device *counter,
switch (u8_get_bits(priv->cmr[count->id], COUNT_MODE)) {
case RANGE_LIMIT:
case MODULO_N:
+ priv->preset[count->id] = ceiling;
quad8_preset_register_set(priv, count->id, ceiling);
spin_unlock_irqrestore(&priv->lock, irqflags);
return 0;
@@ -960,24 +951,28 @@ static int quad8_signal_fck_prescaler_read(struct counter_device *counter,
return 0;
}
+static void quad8_filter_clock_prescaler_set(struct quad8 *const priv, const size_t id,
+ const u8 prescaler)
+{
+ struct channel_reg __iomem *const chan = priv->reg->channel + id;
+
+ iowrite8(SELECT_RLD | RESET_BP, &chan->control);
+ iowrite8(prescaler, &chan->data);
+ iowrite8(SELECT_RLD | TRANSFER_PR0_TO_PSC, &chan->control);
+}
+
static int quad8_signal_fck_prescaler_write(struct counter_device *counter,
struct counter_signal *signal,
u8 prescaler)
{
struct quad8 *const priv = counter_priv(counter);
const size_t channel_id = signal->id / 2;
- struct channel_reg __iomem *const chan = priv->reg->channel + channel_id;
unsigned long irqflags;
spin_lock_irqsave(&priv->lock, irqflags);
priv->fck_prescaler[channel_id] = prescaler;
-
- iowrite8(SELECT_RLD | RESET_BP, &chan->control);
-
- /* Set filter clock factor */
- iowrite8(prescaler, &chan->data);
- iowrite8(SELECT_RLD | RESET_BP | TRANSFER_PR0_TO_PSC, &chan->control);
+ quad8_filter_clock_prescaler_set(priv, channel_id, prescaler);
spin_unlock_irqrestore(&priv->lock, irqflags);
@@ -1177,18 +1172,10 @@ static irqreturn_t quad8_irq_handler(int irq, void *private)
static void quad8_init_counter(struct quad8 *const priv, const size_t channel)
{
struct channel_reg __iomem *const chan = priv->reg->channel + channel;
- unsigned long i;
- iowrite8(SELECT_RLD | RESET_BP, &chan->control);
- /* Reset filter clock factor */
- iowrite8(0, &chan->data);
- iowrite8(SELECT_RLD | RESET_BP | TRANSFER_PR0_TO_PSC, &chan->control);
- iowrite8(SELECT_RLD | RESET_BP, &chan->control);
- /* Reset Preset Register */
- for (i = 0; i < 3; i++)
- iowrite8(0x00, &chan->data);
- iowrite8(SELECT_RLD | RESET_BT_CT_CPT_S_IDX, &chan->control);
- iowrite8(SELECT_RLD | RESET_E, &chan->control);
+ quad8_filter_clock_prescaler_set(priv, channel, 0);
+ quad8_preset_register_set(priv, channel, 0);
+ quad8_flag_register_reset(priv, channel);
/* Binary encoding; Normal count; non-quadrature mode */
priv->cmr[channel] = SELECT_CMR | BINARY | u8_encode_bits(NORMAL_COUNT, COUNT_MODE) |
--
2.39.2Hi William,
I love your patch! Yet something to improve:
[auto build test ERROR on 00f4bc5184c19cb33f468f1ea409d70d19f8f502]
url: https://github.com/intel-lab-lkp/linux/commits/William-Breathitt-Gray/counter-104-quad-8-Utilize-bitfield-access-macros/20230324-052655
base: 00f4bc5184c19cb33f468f1ea409d70d19f8f502
patch link: https://lore.kernel.org/r/c0e914e2b2d5876265494df3ca102edcf259f02a.1679605919.git.william.gray%40linaro.org
patch subject: [PATCH v2 3/3] counter: 104-quad-8: Utilize helper functions to handle PR, FLAG and PSC
config: riscv-allmodconfig (https://download.01.org/0day-ci/archive/20230324/202303241128.WBKc4LIy-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/5da9052d37541a9470e1f9b6621d41b52c148b34
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review William-Breathitt-Gray/counter-104-quad-8-Utilize-bitfield-access-macros/20230324-052655
git checkout 5da9052d37541a9470e1f9b6621d41b52c148b34
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=riscv olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303241128.WBKc4LIy-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from drivers/counter/104-quad-8.c:8:
In function 'field_multiplier',
inlined from 'field_mask' at include/linux/bitfield.h:170:17,
inlined from 'u8_encode_bits' at include/linux/bitfield.h:198:1,
inlined from 'u8p_replace_bits' at include/linux/bitfield.h:198:1,
inlined from 'quad8_control_register_update' at drivers/counter/104-quad-8.c:206:2:
>> include/linux/bitfield.h:165:17: error: call to '__bad_mask' declared with attribute error: bad bitfield mask
165 | __bad_mask();
| ^~~~~~~~~~~~
In function 'field_multiplier',
inlined from 'u8_encode_bits' at include/linux/bitfield.h:198:1,
inlined from 'u8p_replace_bits' at include/linux/bitfield.h:198:1,
inlined from 'quad8_control_register_update' at drivers/counter/104-quad-8.c:206:2:
>> include/linux/bitfield.h:165:17: error: call to '__bad_mask' declared with attribute error: bad bitfield mask
165 | __bad_mask();
| ^~~~~~~~~~~~
vim +/__bad_mask +165 include/linux/bitfield.h
e2192de59e457a Johannes Berg 2023-01-18 119
e2192de59e457a Johannes Berg 2023-01-18 120 /**
e2192de59e457a Johannes Berg 2023-01-18 121 * FIELD_PREP_CONST() - prepare a constant bitfield element
e2192de59e457a Johannes Berg 2023-01-18 122 * @_mask: shifted mask defining the field's length and position
e2192de59e457a Johannes Berg 2023-01-18 123 * @_val: value to put in the field
e2192de59e457a Johannes Berg 2023-01-18 124 *
e2192de59e457a Johannes Berg 2023-01-18 125 * FIELD_PREP_CONST() masks and shifts up the value. The result should
e2192de59e457a Johannes Berg 2023-01-18 126 * be combined with other fields of the bitfield using logical OR.
e2192de59e457a Johannes Berg 2023-01-18 127 *
e2192de59e457a Johannes Berg 2023-01-18 128 * Unlike FIELD_PREP() this is a constant expression and can therefore
e2192de59e457a Johannes Berg 2023-01-18 129 * be used in initializers. Error checking is less comfortable for this
e2192de59e457a Johannes Berg 2023-01-18 130 * version, and non-constant masks cannot be used.
e2192de59e457a Johannes Berg 2023-01-18 131 */
e2192de59e457a Johannes Berg 2023-01-18 132 #define FIELD_PREP_CONST(_mask, _val) \
e2192de59e457a Johannes Berg 2023-01-18 133 ( \
e2192de59e457a Johannes Berg 2023-01-18 134 /* mask must be non-zero */ \
e2192de59e457a Johannes Berg 2023-01-18 135 BUILD_BUG_ON_ZERO((_mask) == 0) + \
e2192de59e457a Johannes Berg 2023-01-18 136 /* check if value fits */ \
e2192de59e457a Johannes Berg 2023-01-18 137 BUILD_BUG_ON_ZERO(~((_mask) >> __bf_shf(_mask)) & (_val)) + \
e2192de59e457a Johannes Berg 2023-01-18 138 /* check if mask is contiguous */ \
e2192de59e457a Johannes Berg 2023-01-18 139 __BF_CHECK_POW2((_mask) + (1ULL << __bf_shf(_mask))) + \
e2192de59e457a Johannes Berg 2023-01-18 140 /* and create the value */ \
e2192de59e457a Johannes Berg 2023-01-18 141 (((typeof(_mask))(_val) << __bf_shf(_mask)) & (_mask)) \
e2192de59e457a Johannes Berg 2023-01-18 142 )
e2192de59e457a Johannes Berg 2023-01-18 143
3e9b3112ec74f1 Jakub Kicinski 2016-08-31 144 /**
3e9b3112ec74f1 Jakub Kicinski 2016-08-31 145 * FIELD_GET() - extract a bitfield element
3e9b3112ec74f1 Jakub Kicinski 2016-08-31 146 * @_mask: shifted mask defining the field's length and position
7240767450d6d8 Masahiro Yamada 2017-10-03 147 * @_reg: value of entire bitfield
3e9b3112ec74f1 Jakub Kicinski 2016-08-31 148 *
3e9b3112ec74f1 Jakub Kicinski 2016-08-31 149 * FIELD_GET() extracts the field specified by @_mask from the
3e9b3112ec74f1 Jakub Kicinski 2016-08-31 150 * bitfield passed in as @_reg by masking and shifting it down.
3e9b3112ec74f1 Jakub Kicinski 2016-08-31 151 */
3e9b3112ec74f1 Jakub Kicinski 2016-08-31 152 #define FIELD_GET(_mask, _reg) \
3e9b3112ec74f1 Jakub Kicinski 2016-08-31 153 ({ \
3e9b3112ec74f1 Jakub Kicinski 2016-08-31 154 __BF_FIELD_CHECK(_mask, _reg, 0U, "FIELD_GET: "); \
3e9b3112ec74f1 Jakub Kicinski 2016-08-31 155 (typeof(_mask))(((_reg) & (_mask)) >> __bf_shf(_mask)); \
3e9b3112ec74f1 Jakub Kicinski 2016-08-31 156 })
3e9b3112ec74f1 Jakub Kicinski 2016-08-31 157
e7d4a95da86e0b Johannes Berg 2018-06-20 158 extern void __compiletime_error("value doesn't fit into mask")
00b0c9b82663ac Al Viro 2017-12-14 159 __field_overflow(void);
00b0c9b82663ac Al Viro 2017-12-14 160 extern void __compiletime_error("bad bitfield mask")
00b0c9b82663ac Al Viro 2017-12-14 161 __bad_mask(void);
00b0c9b82663ac Al Viro 2017-12-14 162 static __always_inline u64 field_multiplier(u64 field)
00b0c9b82663ac Al Viro 2017-12-14 163 {
00b0c9b82663ac Al Viro 2017-12-14 164 if ((field | (field - 1)) & ((field | (field - 1)) + 1))
00b0c9b82663ac Al Viro 2017-12-14 @165 __bad_mask();
00b0c9b82663ac Al Viro 2017-12-14 166 return field & -field;
00b0c9b82663ac Al Viro 2017-12-14 167 }
00b0c9b82663ac Al Viro 2017-12-14 168 static __always_inline u64 field_mask(u64 field)
00b0c9b82663ac Al Viro 2017-12-14 169 {
00b0c9b82663ac Al Viro 2017-12-14 170 return field / field_multiplier(field);
00b0c9b82663ac Al Viro 2017-12-14 171 }
e31a50162feb35 Alex Elder 2020-03-12 172 #define field_max(field) ((typeof(field))field_mask(field))
00b0c9b82663ac Al Viro 2017-12-14 173 #define ____MAKE_OP(type,base,to,from) \
00b0c9b82663ac Al Viro 2017-12-14 174 static __always_inline __##type type##_encode_bits(base v, base field) \
00b0c9b82663ac Al Viro 2017-12-14 175 { \
e7d4a95da86e0b Johannes Berg 2018-06-20 176 if (__builtin_constant_p(v) && (v & ~field_mask(field))) \
00b0c9b82663ac Al Viro 2017-12-14 177 __field_overflow(); \
00b0c9b82663ac Al Viro 2017-12-14 178 return to((v & field_mask(field)) * field_multiplier(field)); \
00b0c9b82663ac Al Viro 2017-12-14 179 } \
00b0c9b82663ac Al Viro 2017-12-14 180 static __always_inline __##type type##_replace_bits(__##type old, \
00b0c9b82663ac Al Viro 2017-12-14 181 base val, base field) \
00b0c9b82663ac Al Viro 2017-12-14 182 { \
00b0c9b82663ac Al Viro 2017-12-14 183 return (old & ~to(field)) | type##_encode_bits(val, field); \
00b0c9b82663ac Al Viro 2017-12-14 184 } \
00b0c9b82663ac Al Viro 2017-12-14 185 static __always_inline void type##p_replace_bits(__##type *p, \
00b0c9b82663ac Al Viro 2017-12-14 186 base val, base field) \
00b0c9b82663ac Al Viro 2017-12-14 187 { \
00b0c9b82663ac Al Viro 2017-12-14 188 *p = (*p & ~to(field)) | type##_encode_bits(val, field); \
00b0c9b82663ac Al Viro 2017-12-14 189 } \
00b0c9b82663ac Al Viro 2017-12-14 190 static __always_inline base type##_get_bits(__##type v, base field) \
00b0c9b82663ac Al Viro 2017-12-14 191 { \
00b0c9b82663ac Al Viro 2017-12-14 192 return (from(v) & field)/field_multiplier(field); \
00b0c9b82663ac Al Viro 2017-12-14 193 }
00b0c9b82663ac Al Viro 2017-12-14 194 #define __MAKE_OP(size) \
00b0c9b82663ac Al Viro 2017-12-14 195 ____MAKE_OP(le##size,u##size,cpu_to_le##size,le##size##_to_cpu) \
00b0c9b82663ac Al Viro 2017-12-14 196 ____MAKE_OP(be##size,u##size,cpu_to_be##size,be##size##_to_cpu) \
00b0c9b82663ac Al Viro 2017-12-14 197 ____MAKE_OP(u##size,u##size,,)
37a3862e123826 Johannes Berg 2018-06-20 198 ____MAKE_OP(u8,u8,,)
00b0c9b82663ac Al Viro 2017-12-14 199 __MAKE_OP(16)
00b0c9b82663ac Al Viro 2017-12-14 200 __MAKE_OP(32)
00b0c9b82663ac Al Viro 2017-12-14 201 __MAKE_OP(64)
00b0c9b82663ac Al Viro 2017-12-14 202 #undef __MAKE_OP
00b0c9b82663ac Al Viro 2017-12-14 203 #undef ____MAKE_OP
00b0c9b82663ac Al Viro 2017-12-14 204
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
© 2016 - 2026 Red Hat, Inc.