lkml.org 
[lkml]   [2021]   [Mar]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v3 3/3] pinctrl: pinctrl-single: fix pcs_pin_dbg_show() when bits_per_mux is not zero
From
Date


On 3/18/2021 2:15 PM, Andy Shevchenko wrote:
>
>
> On Wed, Mar 17, 2021 at 11:42 PM Hanna Hawa<hhhawa@amazon.com> wrote:
>> An SError was detected when trying to print the supported pins in a
> What is SError? Yes, I have read a discussion, but here is the hint:
> if a person sees this as a first text due to, for example, bisecting
> an issue, what she/he can get from this cryptic name?

What you suggest?
s/An SError/A kernel-panic/?
Or remove the sentence and keep the below:
"
This change fixes the pcs_pin_dbg_show() in pinctrl-single driver when
bits_per_mux is not zero. In addition move offset calculation and pin
offset in register to common function.
"

>
>> pinctrl device which supports multiple pins per register. This change
>> fixes the pcs_pin_dbg_show() in pinctrl-single driver when bits_per_mux
>> is not zero. In addition move offset calculation and pin offset in
>> register to common function.
> Reviewed-by: Andy Shevchenko<andy.shevchenko@gmail.com>

Thanks

>
>> Fixes: 4e7e8017a80e ("pinctrl: pinctrl-single: enhance to configure multiple pins of different modules")
>> Signed-off-by: Hanna Hawa<hhhawa@amazon.com>
>> ---
>> drivers/pinctrl/pinctrl-single.c | 57 +++++++++++++++++++++-----------
>> 1 file changed, 37 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
>> index f3394517cb2e..4595acf6545e 100644
>> --- a/drivers/pinctrl/pinctrl-single.c
>> +++ b/drivers/pinctrl/pinctrl-single.c
>> @@ -270,20 +270,46 @@ 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 mux_bytes;
>> +
>> + mux_bytes = pcs->width / BITS_PER_BYTE;
> Can be folded to one line.

Ack

>
>> + if (pcs->bits_per_mux) {
>> + unsigned int pin_offset_bytes;
>> +
>> + pin_offset_bytes = (pcs->bits_per_pin * pin) / BITS_PER_BYTE;
>> + return (pin_offset_bytes / mux_bytes) * mux_bytes;
> Side note for the further improvements (in a separate change, because
> I see that you just copied an original code, and after all this is
> just a fix patch): this can be replaced by round down APIs (one which
> works for arbitrary divisors).

Agree, didn't want to change the formula as it's fix patch. (here and
below), this can be taken for further improvements.

>
>> + }
>> +
>> + return pin * mux_bytes;
>> +}
>> +
>> +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;
> Also a side note: I'm wondering if this can be optimized to have less divisions.
>
>> +}
>> +
>> static void pcs_pin_dbg_show(struct pinctrl_dev *pctldev,
>> struct seq_file *s,
>> unsigned pin)
>> {

Thanks,
Hanna

\
 
 \ /
  Last update: 2021-03-19 08:54    [W:0.055 / U:2.624 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site