[1/4] pinctrl: mcp23s08: Do not complain about unsupported params

It is expected that some of these operations won't work on each and
every HW. Previously, even a simple `cat
/sys/kernel/debug/pinctrl/spi1.1/pinconf-pins` caused excessive dmesg
output.

Signed-off-by: Jan Kundrát <jan.kundrat@cesnet.cz>
---
 drivers/pinctrl/pinctrl-mcp23s08.c | 2 --
 1 file changed, 2 deletions(-)

On 26/01/2018 02:22, Jan Kundrát wrote:
> It is expected that some of these operations won't work on each and
> every HW. Previously, even a simple `cat
> /sys/kernel/debug/pinctrl/spi1.1/pinconf-pins` caused excessive dmesg
> output.

Yes it is rather excessive.
Could be downgraded to dev_dbg instead of removed completely?

Reviewed-by: Phil Reid <preid@electromag.com.au>
> 
> Signed-off-by: Jan Kundrát <jan.kundrat@cesnet.cz>
> ---
>   drivers/pinctrl/pinctrl-mcp23s08.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
> index 022307dd4b54..fd9d27e8126e 100644
> --- a/drivers/pinctrl/pinctrl-mcp23s08.c
> +++ b/drivers/pinctrl/pinctrl-mcp23s08.c
> @@ -265,7 +265,6 @@ static int mcp_pinconf_get(struct pinctrl_dev *pctldev, unsigned int pin,
>   		status = (data & BIT(pin)) ? 1 : 0;
>   		break;
>   	default:
> -		dev_err(mcp->dev, "Invalid config param %04x\n", param);
>   		return -ENOTSUPP;
>   	}
>   
> @@ -292,7 +291,6 @@ static int mcp_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
>   			ret = mcp_set_bit(mcp, MCP_GPPU, pin, arg);
>   			break;
>   		default:
> -			dev_err(mcp->dev, "Invalid config param %04x\n", param);
>   			return -ENOTSUPP;
>   		}
>   	}
>

On čtvrtek 7. června 2018 3:42:49 CEST, Phil Reid wrote:
> On 26/01/2018 02:22, Jan Kundrát wrote:
>> It is expected that some of these operations won't work on each and
>> every HW. Previously, even a simple `cat
>> /sys/kernel/debug/pinctrl/spi1.1/pinconf-pins` caused excessive dmesg
>> output.
>
> Yes it is rather excessive.
> Could be downgraded to dev_dbg instead of removed completely?

Let's split this into two halves, one for mcp_pinconf_get() and another one 
for mcp_pinconf_set().

When checking the pinconf settings, it IMHO makes sense to ask for all 
possible properties. That's what the generic code in debugs is doing, after 
all, and that's why a `cat /sys/kernel/debug/pinctrl/spi*/pinconf-pins` 
currently prints to the kernel log. I think that this should just return 
ENOTSUPP without any extra messages.

I can change the patch to still print out a debugging message when 
attempting to *set* some unsupported configuration. I don't see much value 
in that, but if that's what people want, I'll post an update. Should I do 
that?

Cheers,
Jan
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 7/06/2018 23:30, Jan Kundrát wrote:
> On čtvrtek 7. června 2018 3:42:49 CEST, Phil Reid wrote:
>> On 26/01/2018 02:22, Jan Kundrát wrote:
>>> It is expected that some of these operations won't work on each and
>>> every HW. Previously, even a simple `cat
>>> /sys/kernel/debug/pinctrl/spi1.1/pinconf-pins` caused excessive dmesg
>>> output.
>>
>> Yes it is rather excessive.
>> Could be downgraded to dev_dbg instead of removed completely?
> 
> Let's split this into two halves, one for mcp_pinconf_get() and another one for mcp_pinconf_set().
> 
> When checking the pinconf settings, it IMHO makes sense to ask for all possible properties. That's what the generic code in debugs is doing, after all, and 
> that's why a `cat /sys/kernel/debug/pinctrl/spi*/pinconf-pins` currently prints to the kernel log. I think that this should just return ENOTSUPP without any 
> extra messages.
> 
That makes sense.

> I can change the patch to still print out a debugging message when attempting to *set* some unsupported configuration. I don't see much value in that, but if 
> that's what people want, I'll post an update. Should I do that?
> 
I'm not sure. I always find it nice to see a message when I do something stupid.

On Thu, Jun 7, 2018 at 5:30 PM, Jan Kundrát <jan.kundrat@cesnet.cz> wrote:
> On čtvrtek 7. června 2018 3:42:49 CEST, Phil Reid wrote:
>>
>> On 26/01/2018 02:22, Jan Kundrát wrote:
>>>
>>> It is expected that some of these operations won't work on each and
>>> every HW. Previously, even a simple `cat
>>> /sys/kernel/debug/pinctrl/spi1.1/pinconf-pins` caused excessive dmesg
>>> output.
>>
>>
>> Yes it is rather excessive.
>> Could be downgraded to dev_dbg instead of removed completely?
>
>
> Let's split this into two halves, one for mcp_pinconf_get() and another one
> for mcp_pinconf_set().

It's fine to keep in one patch.

Just update to degrade to dev_dbg() in the common path.

> I can change the patch to still print out a debugging message when
> attempting to *set* some unsupported configuration. I don't see much value
> in that, but if that's what people want, I'll post an update. Should I do
> that?

Yeah that's fine.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html