On Mon, Mar 28, 2022 at 7:45 PM Paul Lemmermann
<[email protected]> wrote:
>
> Fixed case statement issues and spacing issues.
...
> switch (val[j]) {
> - case -1: str = "-1"; break;
> - case 0: str = "0"; break;
> - case 1: str = "1"; break;
> - default: goto unknown_format;
> + case -1:
> + str = "-1";
> + break;
> + case 0:
> + str = "0";
> + break;
> + case 1:
> + str = "1";
> + break;
> + default:
> + goto unknown_format;
> }
What you cited from documentation mostly affects the new code, but
this code is already in the kernel and modifying it, esp. taking into
account 3x LOCs count, looks like an unneeded churn, even if
documentation thinks otherwise.
What I could acknowledge from your proposal is the default case.
Otherwise just leave it to the point if we touch this code for
something else in the future.
...
> static const struct of_device_id kxsd9_of_match[] = {
> - { .compatible = "kionix,kxsd9" },
> - { },
> + { .compatible = "kionix,kxsd9" },
> + { },
I dunno why you touched this line (likely TABs vs. spaces), but please
remove the comma as well here.
> };
--
With Best Regards,
Andy Shevchenko
On Mon, Mar 28, 2022 at 11:32:44PM +0300, Andy Shevchenko wrote:
> On Mon, Mar 28, 2022 at 7:45 PM Paul Lemmermann
> <[email protected]> wrote:
> >
> > Fixed case statement issues and spacing issues.
>
> ...
>
> > switch (val[j]) {
> > - case -1: str = "-1"; break;
> > - case 0: str = "0"; break;
> > - case 1: str = "1"; break;
> > - default: goto unknown_format;
> > + case -1:
> > + str = "-1";
> > + break;
> > + case 0:
> > + str = "0";
> > + break;
> > + case 1:
> > + str = "1";
> > + break;
> > + default:
> > + goto unknown_format;
> > }
>
> What you cited from documentation mostly affects the new code, but
> this code is already in the kernel and modifying it, esp. taking into
> account 3x LOCs count, looks like an unneeded churn, even if
> documentation thinks otherwise.
>
> What I could acknowledge from your proposal is the default case.
> Otherwise just leave it to the point if we touch this code for
> something else in the future.
>
> ...
>
> > static const struct of_device_id kxsd9_of_match[] = {
> > - { .compatible = "kionix,kxsd9" },
> > - { },
> > + { .compatible = "kionix,kxsd9" },
>
> > + { },
>
> I dunno why you touched this line (likely TABs vs. spaces), but please
> remove the comma as well here.
Yes, that is exactly why this patch exists.
Thank you, I will take these thoughts into account and submit a new
patch.
Thanks!
Paul
>
> > };
>
> --
> With Best Regards,
> Andy Shevchenko