On 3/17/2021 2:27 PM, Andy Shevchenko wrote:
On Tue, Mar 16, 2021 at 11:24 PM Hanna Hawa <hhhawa@xxxxxxxxxx> wrote:
An SError was detected when trying to print the supported pins in a
What SError is?
System error:
[ 24.257831] SError Interrupt on CPU0, code 0xbf000002 -- SError
...
[ 24.257855] Kernel panic - not syncing: Asynchronous SError Interrupt
pinctrl device which supports multiple pins per register. This change
fixes the pcs_pin_dbg_show() in pinctrl-single driver when
bits_per_mux != 0. In addition move offset calculation and pin offset in
'!= 0' -> 'is not zero'
Will be fixed.
register to common function.
Fixes tag?
Sure, will be added.
Signed-off-by: Hanna Hawa <hhhawa@xxxxxxxxxx>
---
drivers/pinctrl/pinctrl-single.c | 66 ++++++++++++++++++++++----------
1 file changed, 45 insertions(+), 21 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index f3394517cb2e..434f90c8b1b3 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -270,20 +270,53 @@ static void __maybe_unused pcs_writel(unsigned val, void __iomem *reg)
writel(val, reg);
}
+static unsigned int pcs_pin_reg_offset_get(struct pcs_device *pcs,
+ unsigned int pin)
+{
+ unsigned int offset, mux_bytes;
Offset can be replaced by direct return statements.
Ack.
+ mux_bytes = pcs->width / BITS_PER_BYTE;
+
+ if (pcs->bits_per_mux) {
+ unsigned int pin_offset_bytes;
+
+ pin_offset_bytes = (pcs->bits_per_pin * pin) / BITS_PER_BYTE;
+ offset = (pin_offset_bytes / mux_bytes) * mux_bytes;
+ } else {
+ offset = pin * mux_bytes;
+ }
+
+ return offset;
+}
+
+static unsigned int pcs_pin_shift_reg_get(struct pcs_device *pcs,
+ unsigned int pin)
+{
+ return ((pin % (pcs->width / pcs->bits_per_pin)) * pcs->bits_per_pin);
Too many parentheses.
Will remove the extra parentheses.
+}
+
static void pcs_pin_dbg_show(struct pinctrl_dev *pctldev,
struct seq_file *s,
unsigned pin)
{
struct pcs_device *pcs;
- unsigned val, mux_bytes;
+ unsigned int val;
unsigned long offset;
size_t pa;
pcs = pinctrl_dev_get_drvdata(pctldev);
- mux_bytes = pcs->width / BITS_PER_BYTE;
- offset = pin * mux_bytes;
- val = pcs->read(pcs->base + offset);
+ offset = pcs_pin_reg_offset_get(pcs, pin);
+
+ if (pcs->bits_per_mux) {
+ unsigned int pin_shift_in_reg = pcs_pin_shift_reg_get(pcs, pin);
+ val = pcs->read(pcs->base + offset)
+ & (pcs->fmask << pin_shift_in_reg);
One line?
At least move & to the upper line.
+ } else {
+ val = pcs->read(pcs->base + offset);
It's the same as in above branch, why not
val = read();
if ()
val &= fmask << _reg_get(...);
?
Agree, looks better
Thanks Andy, will send new patchset soon.
Thanks,
Hanna
