diff mbox series

[v3,07/11] power: supply: core: tabularize HWMON temperature labels

Message ID 29b5043db9a51ef7a0cb6e3a8c69c91e36045cd6.1585944770.git.mirq-linux@rere.qmqm.pl (mailing list archive)
State Not Applicable, archived
Headers show
Series extensions and fixes | expand

Commit Message

Michał Mirosław April 3, 2020, 8:20 p.m. UTC
Rework power_supply_hwmon_read_string() to check it's parameters.
This allows to extend it later with labels for other types of
measurements.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
v2: split from fix temperature labels
v3: remove power_supply_hwmon_read_string() parameter checks
    as it is internal API (suggested by Guenter Roeck)
---
 drivers/power/supply/power_supply_hwmon.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

Comments

kernel test robot April 5, 2020, 1:52 a.m. UTC | #1
Hi "Michał,

I love your patch! Perhaps something to improve:

[auto build test WARNING on power-supply/for-next]
[also build test WARNING on hwmon/hwmon-next linus/master v5.6 next-20200404]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Micha-Miros-aw/extensions-and-fixes/20200405-044024
base:   https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
config: x86_64-randconfig-b002-20200405 (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 62f3a9650a9f289a07a5f480764fb655178c2334)
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/power/supply/power_supply_hwmon.o: warning: objtool: power_supply_hwmon_read_string() falls through to next function power_supply_hwmon_write()

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Nick Desaulniers April 7, 2020, 6:13 p.m. UTC | #2
On Sat, Apr 4, 2020 at 6:53 PM kbuild test robot <lkp@intel.com> wrote:
>
> Hi "Michał,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on power-supply/for-next]
> [also build test WARNING on hwmon/hwmon-next linus/master v5.6 next-20200404]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
>
> url:    https://github.com/0day-ci/linux/commits/Micha-Miros-aw/extensions-and-fixes/20200405-044024
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
> config: x86_64-randconfig-b002-20200405 (attached as .config)
> compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 62f3a9650a9f289a07a5f480764fb655178c2334)
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         COMPILER=clang make.cross ARCH=x86_64
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kbuild test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
> >> drivers/power/supply/power_supply_hwmon.o: warning: objtool: power_supply_hwmon_read_string() falls through to next function power_supply_hwmon_write()

I'm guessing this is from the unreachable:
https://github.com/0day-ci/linux/commit/b8b2d14ca46ca54257f55c9af58ea25695b9ee36
I'll need to play with this some more as I couldn't reproduce with a
simplified test case, but looks like a compiler bug.  Filed
https://github.com/ClangBuiltLinux/linux/issues/978 for me to track.
Michał Mirosław April 7, 2020, 7:55 p.m. UTC | #3
On Tue, Apr 07, 2020 at 11:13:50AM -0700, Nick Desaulniers wrote:
> On Sat, Apr 4, 2020 at 6:53 PM kbuild test robot <lkp@intel.com> wrote:
> >
> > Hi "Michał,
> >
> > I love your patch! Perhaps something to improve:
> >
> > [auto build test WARNING on power-supply/for-next]
> > [also build test WARNING on hwmon/hwmon-next linus/master v5.6 next-20200404]
> > [if your patch is applied to the wrong git tree, please drop us a note to help
> > improve the system. BTW, we also suggest to use '--base' option to specify the
> > base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> >
> > url:    https://github.com/0day-ci/linux/commits/Micha-Miros-aw/extensions-and-fixes/20200405-044024
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
> > config: x86_64-randconfig-b002-20200405 (attached as .config)
> > compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 62f3a9650a9f289a07a5f480764fb655178c2334)
> > reproduce:
> >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # save the attached .config to linux build tree
> >         COMPILER=clang make.cross ARCH=x86_64
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kbuild test robot <lkp@intel.com>
> >
> > All warnings (new ones prefixed by >>):
> >
> > >> drivers/power/supply/power_supply_hwmon.o: warning: objtool: power_supply_hwmon_read_string() falls through to next function power_supply_hwmon_write()
> 
> I'm guessing this is from the unreachable:
> https://github.com/0day-ci/linux/commit/b8b2d14ca46ca54257f55c9af58ea25695b9ee36
> I'll need to play with this some more as I couldn't reproduce with a
> simplified test case, but looks like a compiler bug.  Filed
> https://github.com/ClangBuiltLinux/linux/issues/978 for me to track.

Hi Nick,

Just guessing: have you tried adding another unrelated function to the
testcase? I would expect that 'fall through to next function' needs
some other function to match.

Best Regards,
Michał Mirosław
Nick Desaulniers April 7, 2020, 7:57 p.m. UTC | #4
On Tue, Apr 7, 2020 at 12:56 PM Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
>
> On Tue, Apr 07, 2020 at 11:13:50AM -0700, Nick Desaulniers wrote:
> > On Sat, Apr 4, 2020 at 6:53 PM kbuild test robot <lkp@intel.com> wrote:
> > >
> > > Hi "Michał,
> > >
> > > I love your patch! Perhaps something to improve:
> > >
> > > [auto build test WARNING on power-supply/for-next]
> > > [also build test WARNING on hwmon/hwmon-next linus/master v5.6 next-20200404]
> > > [if your patch is applied to the wrong git tree, please drop us a note to help
> > > improve the system. BTW, we also suggest to use '--base' option to specify the
> > > base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> > >
> > > url:    https://github.com/0day-ci/linux/commits/Micha-Miros-aw/extensions-and-fixes/20200405-044024
> > > base:   https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
> > > config: x86_64-randconfig-b002-20200405 (attached as .config)
> > > compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 62f3a9650a9f289a07a5f480764fb655178c2334)
> > > reproduce:
> > >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > >         chmod +x ~/bin/make.cross
> > >         # save the attached .config to linux build tree
> > >         COMPILER=clang make.cross ARCH=x86_64
> > >
> > > If you fix the issue, kindly add following tag as appropriate
> > > Reported-by: kbuild test robot <lkp@intel.com>
> > >
> > > All warnings (new ones prefixed by >>):
> > >
> > > >> drivers/power/supply/power_supply_hwmon.o: warning: objtool: power_supply_hwmon_read_string() falls through to next function power_supply_hwmon_write()
> >
> > I'm guessing this is from the unreachable:
> > https://github.com/0day-ci/linux/commit/b8b2d14ca46ca54257f55c9af58ea25695b9ee36
> > I'll need to play with this some more as I couldn't reproduce with a
> > simplified test case, but looks like a compiler bug.  Filed
> > https://github.com/ClangBuiltLinux/linux/issues/978 for me to track.
>
> Hi Nick,
>
> Just guessing: have you tried adding another unrelated function to the
> testcase? I would expect that 'fall through to next function' needs
> some other function to match.

I was throwing the test case linked into godbolt and looking at the
generated assembly.  It contained no jumps, only movs and conditional
movs.  Thank you for the suggestion.
Nick Desaulniers April 7, 2020, 10:31 p.m. UTC | #5
+ Ilie

On Tue, Apr 7, 2020 at 12:57 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Tue, Apr 7, 2020 at 12:56 PM Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> >
> > On Tue, Apr 07, 2020 at 11:13:50AM -0700, Nick Desaulniers wrote:
> > > On Sat, Apr 4, 2020 at 6:53 PM kbuild test robot <lkp@intel.com> wrote:
> > > >
> > > > Hi "Michał,
> > > >
> > > > I love your patch! Perhaps something to improve:
> > > >
> > > > [auto build test WARNING on power-supply/for-next]
> > > > [also build test WARNING on hwmon/hwmon-next linus/master v5.6 next-20200404]
> > > > [if your patch is applied to the wrong git tree, please drop us a note to help
> > > > improve the system. BTW, we also suggest to use '--base' option to specify the
> > > > base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> > > >
> > > > url:    https://github.com/0day-ci/linux/commits/Micha-Miros-aw/extensions-and-fixes/20200405-044024
> > > > base:   https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
> > > > config: x86_64-randconfig-b002-20200405 (attached as .config)
> > > > compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 62f3a9650a9f289a07a5f480764fb655178c2334)
> > > > reproduce:
> > > >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > > >         chmod +x ~/bin/make.cross
> > > >         # save the attached .config to linux build tree
> > > >         COMPILER=clang make.cross ARCH=x86_64
> > > >
> > > > If you fix the issue, kindly add following tag as appropriate
> > > > Reported-by: kbuild test robot <lkp@intel.com>
> > > >
> > > > All warnings (new ones prefixed by >>):
> > > >
> > > > >> drivers/power/supply/power_supply_hwmon.o: warning: objtool: power_supply_hwmon_read_string() falls through to next function power_supply_hwmon_write()
> > >
> > > I'm guessing this is from the unreachable:
> > > https://github.com/0day-ci/linux/commit/b8b2d14ca46ca54257f55c9af58ea25695b9ee36
> > > I'll need to play with this some more as I couldn't reproduce with a
> > > simplified test case, but looks like a compiler bug.  Filed
> > > https://github.com/ClangBuiltLinux/linux/issues/978 for me to track.
> >
> > Hi Nick,
> >
> > Just guessing: have you tried adding another unrelated function to the
> > testcase? I would expect that 'fall through to next function' needs
> > some other function to match.

See Ilie's suggestion:
https://github.com/ClangBuiltLinux/linux/issues/978#issuecomment-610633039
Michał Mirosław April 20, 2020, 9:22 a.m. UTC | #6
On Tue, Apr 07, 2020 at 11:13:50AM -0700, Nick Desaulniers wrote:
> On Sat, Apr 4, 2020 at 6:53 PM kbuild test robot <lkp@intel.com> wrote:
> >
> > Hi "Michał,
> >
> > I love your patch! Perhaps something to improve:
> >
> > [auto build test WARNING on power-supply/for-next]
> > [also build test WARNING on hwmon/hwmon-next linus/master v5.6 next-20200404]
> > [if your patch is applied to the wrong git tree, please drop us a note to help
> > improve the system. BTW, we also suggest to use '--base' option to specify the
> > base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> >
> > url:    https://github.com/0day-ci/linux/commits/Micha-Miros-aw/extensions-and-fixes/20200405-044024
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
> > config: x86_64-randconfig-b002-20200405 (attached as .config)
> > compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 62f3a9650a9f289a07a5f480764fb655178c2334)
> > reproduce:
> >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # save the attached .config to linux build tree
> >         COMPILER=clang make.cross ARCH=x86_64
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kbuild test robot <lkp@intel.com>
> >
> > All warnings (new ones prefixed by >>):
> >
> > >> drivers/power/supply/power_supply_hwmon.o: warning: objtool: power_supply_hwmon_read_string() falls through to next function power_supply_hwmon_write()
> 
> I'm guessing this is from the unreachable:
> https://github.com/0day-ci/linux/commit/b8b2d14ca46ca54257f55c9af58ea25695b9ee36
> I'll need to play with this some more as I couldn't reproduce with a
> simplified test case, but looks like a compiler bug.  Filed
> https://github.com/ClangBuiltLinux/linux/issues/978 for me to track.

Hi,

For gcc this is bug 51513 [1]. This does not affect correctness of the
code, so I wonder if we should/need be trying to work around it.

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51513

Best Regards,
Michał Mirosław
Sebastian Reichel May 1, 2020, 12:47 p.m. UTC | #7
Hi,

On Mon, Apr 20, 2020 at 11:22:09AM +0200, Michał Mirosław wrote:
> On Tue, Apr 07, 2020 at 11:13:50AM -0700, Nick Desaulniers wrote:
> > On Sat, Apr 4, 2020 at 6:53 PM kbuild test robot <lkp@intel.com> wrote:
> > >
> > > Hi "Michał,
> > >
> > > I love your patch! Perhaps something to improve:
> > >
> > > [auto build test WARNING on power-supply/for-next]
> > > [also build test WARNING on hwmon/hwmon-next linus/master v5.6 next-20200404]
> > > [if your patch is applied to the wrong git tree, please drop us a note to help
> > > improve the system. BTW, we also suggest to use '--base' option to specify the
> > > base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> > >
> > > url:    https://github.com/0day-ci/linux/commits/Micha-Miros-aw/extensions-and-fixes/20200405-044024
> > > base:   https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
> > > config: x86_64-randconfig-b002-20200405 (attached as .config)
> > > compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 62f3a9650a9f289a07a5f480764fb655178c2334)
> > > reproduce:
> > >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > >         chmod +x ~/bin/make.cross
> > >         # save the attached .config to linux build tree
> > >         COMPILER=clang make.cross ARCH=x86_64
> > >
> > > If you fix the issue, kindly add following tag as appropriate
> > > Reported-by: kbuild test robot <lkp@intel.com>
> > >
> > > All warnings (new ones prefixed by >>):
> > >
> > > >> drivers/power/supply/power_supply_hwmon.o: warning: objtool: power_supply_hwmon_read_string() falls through to next function power_supply_hwmon_write()
> > 
> > I'm guessing this is from the unreachable:
> > https://github.com/0day-ci/linux/commit/b8b2d14ca46ca54257f55c9af58ea25695b9ee36
> > I'll need to play with this some more as I couldn't reproduce with a
> > simplified test case, but looks like a compiler bug.  Filed
> > https://github.com/ClangBuiltLinux/linux/issues/978 for me to track.
> 
> Hi,
> 
> For gcc this is bug 51513 [1]. This does not affect correctness of the
> code, so I wonder if we should/need be trying to work around it.
> 
> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51513

Yes, we need to work around it for now. If I understand the situation
correctly, a simple workaround would be:

default:
    /*
     * unreachable, but not explicitly marked because this triggers
     * a compiler bug in gcc and llvm:
     *
     *  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51513
     *  https://github.com/ClangBuiltLinux/linux/issues/978
     */
    break;

-- Sebastian
diff mbox series

Patch

diff --git a/drivers/power/supply/power_supply_hwmon.c b/drivers/power/supply/power_supply_hwmon.c
index 67b6ee60085e..5621e72a39f0 100644
--- a/drivers/power/supply/power_supply_hwmon.c
+++ b/drivers/power/supply/power_supply_hwmon.c
@@ -13,6 +13,11 @@  struct power_supply_hwmon {
 	unsigned long *props;
 };
 
+static const char *const ps_temp_label[] = {
+	"temp",
+	"ambient temp",
+};
+
 static int power_supply_hwmon_in_to_property(u32 attr)
 {
 	switch (attr) {
@@ -144,7 +149,14 @@  static int power_supply_hwmon_read_string(struct device *dev,
 					  u32 attr, int channel,
 					  const char **str)
 {
-	*str = channel ? "temp ambient" : "temp";
+	switch (type) {
+	case hwmon_temp:
+		*str = ps_temp_label[channel];
+		break;
+	default:
+		unreachable();
+	}
+
 	return 0;
 }