Re: [PATCH v5 2/2] pinctrl: qcom: Add sm8250 lpass lpi pinctrl driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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.



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux