Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <1680076012-10785-1-git-send-email-quic_mojha@quicinc.com>
 <1680076012-10785-6-git-send-email-quic_mojha@quicinc.com>
 <ZHEvWpCqg_oyWyZW@surfacebook> <20230526233604.3eiqbfrkecir3ga5@ripper>
In-Reply-To: <20230526233604.3eiqbfrkecir3ga5@ripper>
From:   Andy Shevchenko <andy.shevchenko@gmail.com>
Date:   Sat, 27 May 2023 10:22:13 +0300
Message-ID: <CAHp75VfcvNpdSP_XvbB42ZUxCik3v-jhR7WFjsaJVr3AZR=nJg@mail.gmail.com>
Subject: Re: [PATCH v6 5/5] firmware: qcom_scm: Add multiple download mode support
To:     Bjorn Andersson <andersson@kernel.org>
Cc:     Mukesh Ojha <quic_mojha@quicinc.com>, agross@kernel.org,
        konrad.dybcio@linaro.org, linus.walleij@linaro.org,
        linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-gpio@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Sat, May 27, 2023 at 2:32=E2=80=AFAM Bjorn Andersson <andersson@kernel.o=
rg> wrote:
> On Sat, May 27, 2023 at 01:14:50AM +0300, andy.shevchenko@gmail.com wrote=
:
> > Wed, Mar 29, 2023 at 01:16:52PM +0530, Mukesh Ojha kirjoitti:

...

> > >     if (download_mode =3D=3D QCOM_DOWNLOAD_FULLDUMP)
> > >             len =3D sysfs_emit(buffer, "full\n");
> > > +   else if (download_mode =3D=3D QCOM_DOWNLOAD_MINIDUMP)
> > > +           len =3D sysfs_emit(buffer, "mini\n");
> > > +   else if (download_mode =3D=3D QCOM_DOWNLOAD_BOTHDUMP)
> >
> > > +           len =3D sysfs_emit(buffer, "full,mini\n");
> >
> > Why not "both" ?

> "both" isn't very future proof (and I think we've had additional
> variations in the past already), so I asked for this form.

Okay, so this should be the bit flags and we should parse a list of
the values. In that case I may agree with the approach.

> >       if (mode >=3D ARRAY_SIZE(...))
> >               return sysfs_emit("Oh heh!\n");
> >
> >       return sysfs_emit("%s\n", array[mode]);

...

> > > +   if (sysfs_streq(val, "full,mini") || sysfs_streq(val, "mini,full"=
)) {
> > > +           download_mode =3D QCOM_DOWNLOAD_BOTHDUMP;
> >
> > It's way too hard, esp. taking into account that once user enters wrong=
 order,
> > user can't simply validate this by reading value back.
> >
> > Use "both" and that's it.
> >
> > > +   } else if (sysfs_streq(val, "full")) {
> > >             download_mode =3D QCOM_DOWNLOAD_FULLDUMP;
> > > +   } else if (sysfs_streq(val, "mini")) {
> > > +           download_mode =3D QCOM_DOWNLOAD_MINIDUMP;

As per above.

--=20
With Best Regards,
Andy Shevchenko