public inbox for linux-usb@vger.kernel.org
 help / color / mirror / Atom feed
From: Francesco Dolcini <francesco@dolcini.it>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Francesco Dolcini <francesco@dolcini.it>,
	Heikki Krogerus <heikki.krogerus@linux.intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Francesco Dolcini <francesco.dolcini@toradex.com>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	Bjorn Andersson <andersson@kernel.org>
Subject: Re: [PATCH v1 2/2] usb: typec: mux: gpio-sbu: Make enable gpio optional
Date: Fri, 24 May 2024 13:31:58 +0200	[thread overview]
Message-ID: <ZlB6ruZ8j2rVsIio@gaggiata.pivistrello.it> (raw)
In-Reply-To: <5of64nmgpotr7fu66urgko5gfvr4ffhmff4dgkagkdvwh2dywk@etlw6rsmhki6>

Hello Dmitry,
thanks for the review.

On Fri, May 24, 2024 at 12:56:15PM +0300, Dmitry Baryshkov wrote:
> On Fri, May 24, 2024 at 09:10:34AM +0200, Francesco Dolcini wrote:
> > From: Francesco Dolcini <francesco.dolcini@toradex.com>
> > 
> > The enable gpio is not required when the SBU mux is used only for
> > orientation, make it optional.
> > 
> > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > ---
> >  drivers/usb/typec/mux/gpio-sbu-mux.c | 11 ++++++++---
> >  1 file changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/usb/typec/mux/gpio-sbu-mux.c b/drivers/usb/typec/mux/gpio-sbu-mux.c
> > index 374168482d36..cf44259980a1 100644
> > --- a/drivers/usb/typec/mux/gpio-sbu-mux.c
> > +++ b/drivers/usb/typec/mux/gpio-sbu-mux.c
...
> > @@ -66,6 +66,9 @@ static int gpio_sbu_mux_set(struct typec_mux_dev *mux,
> >  {
> >  	struct gpio_sbu_mux *sbu_mux = typec_mux_get_drvdata(mux);
> >  
> > +	if (!sbu_mux->enable_gpio)
> > +		return -EOPNOTSUPP;
> 
> Can we skip registering the mux if there is no enable_gpio? This can
> save users from the unexpected errors during runtime.

Yes, I considered this option.

The rationale for the current implementation is that if the device tree is
correct (no mode-switch property, when enable-gpios is not present), nobody
will call gpio_sbu_mux_set() so no runtime error is possible. If the
configuration in the DT is not correct you get this runtime error.

With your proposal in case the DT configuration is not correct there will be no
errors from the kernel, but the functionality will not work.

Francesco


  reply	other threads:[~2024-05-24 11:32 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-24  7:10 [PATCH v1 0/2] usb: typec: mux: gpio-sbu: Make enable gpio optional Francesco Dolcini
2024-05-24  7:10 ` [PATCH v1 1/2] dt-bindings: usb: gpio-sbu-mux: Make 'enable-gpios' optional Francesco Dolcini
2024-05-24 18:31   ` Conor Dooley
2024-05-24  7:10 ` [PATCH v1 2/2] usb: typec: mux: gpio-sbu: Make enable gpio optional Francesco Dolcini
2024-05-24  9:56   ` Dmitry Baryshkov
2024-05-24 11:31     ` Francesco Dolcini [this message]
2024-05-24 11:42       ` Dmitry Baryshkov
2024-05-24 12:51         ` Francesco Dolcini
2024-05-24 20:25           ` Dmitry Baryshkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZlB6ruZ8j2rVsIio@gaggiata.pivistrello.it \
    --to=francesco@dolcini.it \
    --cc=andersson@kernel.org \
    --cc=dmitry.baryshkov@linaro.org \
    --cc=francesco.dolcini@toradex.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=heikki.krogerus@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox