Re: [PATCH 2/2] HID: mcp2221: configure GP pins for GPIO function

From: rishi gupta <gupt21@gmail.com>
To: Linus Walleij <linus.walleij@linaro.org>,
	Tobias Junghans <tobias.junghans@inhub.de>
Cc: Linux Input <linux-input@vger.kernel.org>,
	Jiri Kosina <jikos@kernel.org>,
	Benjamin Tissoires <benjamin.tissoires@redhat.com>
Subject: Re: [PATCH 2/2] HID: mcp2221: configure GP pins for GPIO function
Date: Sun, 19 Sep 2021 16:12:55 +0530	[thread overview]
Message-ID: <CALUj-gvpwhYW8FuTaEjSmgNUhMTizYKvXDxAu6c6SNPbHNdOyg@mail.gmail.com> (raw)
In-Reply-To: <CACRpkdZSmiVPOY8==pAhnUnYUn5xv6Wf5-tKEEJrpcqPF47++w@mail.gmail.com>

Thanks Linus.

Hi Tobias, few observations on code:
1. Copy-paste error; please change set to get in comments for struct
mcp_get_sram_settings.
2. If mcp_configure_gp() fails, we have invalid copy of
mcp->gp_runtime_settings (new value is not set actually but we have
updated this array with new value)..

Regards,
Rishi

On Fri, Sep 17, 2021 at 4:44 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Wed, Aug 18, 2021 at 5:28 PM Tobias Junghans
> <tobias.junghans@inhub.de> wrote:
>
> > Per default the GP pins of an MCP2221 are designated to a certain
> > dedicated or alternate function. While it's possible to change these
> > defaults by manually updating the internal flash, it's much more
> > convenient and safe to configure the GP pins as GPIOs automatically
> > at runtime whenever the corresponding GPIO line is requested. The
> > previous setting is restored as soon as the GPIO line is freed again.
> >
> > Signed-off-by: Tobias Junghans <tobias.junghans@inhub.de>
>
> My sympathies are usually on the side of users trying to make
> use of their hardware and they should be able to.
>
> For other wrong configured GPIO expanders such as FTDI
> a publicly available firmware reflash tool exists, and if that does
> not exist for this hardware, I think this approach is legitimate.
>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>
> Yours,
> Linus Walleij