On 02/12/2020 09:56, Srinivas Kandagatla wrote:
+ case PIN_CONFIG_SLEW_RATE:
+ if (arg > LPI_SLEW_RATE_MAX) {
+ dev_err(pctldev->dev, "invalid slew rate %u for pin:
%d\n",
+ arg, group);
+ return -EINVAL;
+ }
+
+ slew_offset = g->slew_offset;
+ if (slew_offset == NO_SLEW)
+ break;
+
+ mutex_lock(&pctrl->slew_access_lock);
+ sval = ioread32(pctrl->slew_base + LPI_SLEW_RATE_CTL_REG);
+
+ for (i = 0; i < LPI_SLEW_BITS_SIZE; i++) {
+ assign_bit(slew_offset, &sval, arg & 0x01);
+ slew_offset++;
+ arg = arg >> 1;
+ }
Isn't this loop just the same as
FIELD_SET(3 << slew_offset, arg & 3, sval)
None of FIELD_* or replace_bits apis will work here, as the mask passed
to those macros should be a constant #define. Passing variable to them
in mask will result in compile error!
mask in this case is retrieved at runtime.
I think we should live with the existing code unless there is a strong
reason for it to change! Or a better alternative.
--srini
This will not work FIELD_SET will not clear any bits wich are already
set! assing_bit will work, but we could do better by adding slew_mask
per pin rather than slew_offset which should allow us to use
replace_bits straight away.
