Re: [PATCH 9/9] soc/qcom: Add REVID driver
From: Konrad Dybcio
Date: Sun Jul 26 2020 - 07:41:28 EST
Hi Greg, thanks for your review!
>Why do we need this noise in the kernel log?
I guess it could be left there as a debug print? Knowing your hardware
revision seems like a good, but yeah, not a necessary thing.
>You can drop the GPL boilerplate text and add a proper SPDX line at the
Seems I only did that in my other local tree.. whoops!
>Drivers should always use dev_err() and friends, as you have access to a
>struct device * always. Please fix up the driver here to use that api
>instead, no pr_* should be needed at all.
>Horrible global symbol name. Who calls this?
Welcome to development on qcom platforms :D
>This is the last patch in
>the series, so if there is no user for this, please don't export it.
Other downstream drivers make use of it.. need to get this up first, sorry :V
>Why do you need a .h file in the include directory if only a single .c
>file needs it? Just put that info in the .c file itself.
Again, other downstream drivers which some people and I intend to
bring to upstream standards use that to access the PMIC model/hw revision.
>But again, who uses this module? If it's only good for a single line in
>the kernel log, that feels like a huge waste to me.
downstream-kernel-dir$ rg -l qpnp-revid.h | wc -l
So yeah, quite a bunch of other qcom-specific drivers.
I'll try to fix these and send a v2.