Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1430801

Re: [PATCH 1/5] hwmon: (tmp102) Use devm_add_action to register cleanup function

From Guenter Roeck <linux@roeck-us.net>
Newsgroups linux.kernel
Subject Re: [PATCH 1/5] hwmon: (tmp102) Use devm_add_action to register cleanup function
Date 2016-06-24 18:40 +0200
Message-ID <rNCj8-22X-13@gated-at.bofh.it> (permalink)
References <rNnap-QK-1@gated-at.bofh.it> <rNA7D-Hi-9@gated-at.bofh.it> <rNAr1-NX-45@gated-at.bofh.it> <rNAKl-UD-5@gated-at.bofh.it> <rNBdn-1jE-15@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Fri, Jun 24, 2016 at 10:23:10AM -0500, Nishanth Menon wrote:
> On 06/24/2016 09:54 AM, Guenter Roeck wrote:
> > On 06/24/2016 07:30 AM, Guenter Roeck wrote:
> >> Hi Nishanth,
> >>
> >> On 06/24/2016 07:13 AM, Nishanth Menon wrote:
> >>> On 06/23/2016 07:28 PM, Guenter Roeck wrote:
> >>>> By registering a cleanup function with devm_add_action(), we can
> >>>> simplify the error path in the probe function and drop the remove
> >>>> function entirely.
> >>>>
> >>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> >>>
> >>> I dont seem to have a cover letter to reply to... but anyways..
> >>>
> >>> Before: http://pastebin.ubuntu.com/17801376/
> >>> After all 5 patches: http://pastebin.ubuntu.com/17801824/
> >>>
> >>> Fails on beagleboard-X15 with:
> >>> [   36.781509] tmp102 0-0048: No cache defaults, reading back from HW
> >>> [   36.795940] tmp102 0-0048: unexpected config register value
> >>>
> >>> I have'nt bisected down on the specific patch in the series. Have you had a chance to test the series?
> >>>
> >>>
> >>
> >> Thanks for testing. Yes, I did test it. Maybe different chip revisions, or different
> >> initial config register values and I messed up there. Can you send me the output
> >> of i2cdump (word wide) ?
> >>
> > 
> > Before 5 patches:
> >> # i2cdump -f 0 0x48 w
> >>      0,8  1,9  2,a  3,b  4,c  5,d  6,e  7,f
> >> 00: 7912 b062 004b 0050 c018 e006 0000 0000 

[ ... ]
> > 
> > After 5 patches:
> >>  i2cdump -y 0 0x48 w
> >>      0,8  1,9  2,a  3,b  4,c  5,d  6,e  7,f
> >> 00: 5024 a060 004b 0050 c018 e006 0000 0000 

[ .... ]

> I can try and debug the series once I get some spare time, might be
> over the weekend or next week.

The register map, at least the initial one, is pretty much the same as mine
and as expected. The configuration register in the second map is messed up,
possible because of a write with wrong endianness.

I bet the regmap_read() of the configuration register returns 0xa060 (or
0xb062) instead of 0x60a0 / 0x62b0 on your system. If you find the time,
it would be great if you can confirm by printing the register value with
the "unexpected config register value" message (guess I should have left
that in place - I used it during testing ;-).

If that is the case, I'll probably have to drop the regmap changes, at least
for now. It would mean that regmap is broken for chips like the LM75 (ie
for all chips with 16-bit registers) on controllers supporting I2C_FUNC_I2C.

Thanks,
Guenter

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 1/5] hwmon: (tmp102) Use devm_add_action to register cleanup function Guenter Roeck <linux@roeck-us.net> - 2016-06-24 02:30 +0200
  [PATCH 2/5] hwmon: (tmp102) Drop FSF address Guenter Roeck <linux@roeck-us.net> - 2016-06-24 02:30 +0200
  [PATCH 3/5] hwmon: (tmp102) Improve handling of initial read delay Guenter Roeck <linux@roeck-us.net> - 2016-06-24 02:30 +0200
  [PATCH 5/5] hwmon: (tmp102) Convert to use regmap, and drop local cache Guenter Roeck <linux@roeck-us.net> - 2016-06-24 02:30 +0200
  [PATCH 4/5] hwmon: (tmp102) Rework chip configuration Guenter Roeck <linux@roeck-us.net> - 2016-06-24 02:30 +0200
  Re: [PATCH 1/5] hwmon: (tmp102) Use devm_add_action to register  cleanup function Nishanth Menon <nm@ti.com> - 2016-06-24 16:20 +0200
    Re: [PATCH 1/5] hwmon: (tmp102) Use devm_add_action to register  cleanup function Guenter Roeck <linux@roeck-us.net> - 2016-06-24 16:40 +0200
      Re: [PATCH 1/5] hwmon: (tmp102) Use devm_add_action to register  cleanup function Guenter Roeck <linux@roeck-us.net> - 2016-06-24 17:00 +0200
        Re: [PATCH 1/5] hwmon: (tmp102) Use devm_add_action to register  cleanup function Nishanth Menon <nm@ti.com> - 2016-06-24 17:30 +0200
          Re: [PATCH 1/5] hwmon: (tmp102) Use devm_add_action to register  cleanup function Nishanth Menon <nm@ti.com> - 2016-06-24 17:40 +0200
          Re: [PATCH 1/5] hwmon: (tmp102) Use devm_add_action to register  cleanup function Guenter Roeck <linux@roeck-us.net> - 2016-06-24 18:40 +0200
            Re: [PATCH 1/5] hwmon: (tmp102) Use devm_add_action to register  cleanup function Nishanth Menon <nm@ti.com> - 2016-06-24 20:10 +0200
              Re: [PATCH 1/5] hwmon: (tmp102) Use devm_add_action to register  cleanup function Guenter Roeck <linux@roeck-us.net> - 2016-06-24 20:20 +0200
                Re: [PATCH 1/5] hwmon: (tmp102) Use devm_add_action to register  cleanup function Nishanth Menon <nm@ti.com> - 2016-06-24 20:30 +0200
                Re: [PATCH 1/5] hwmon: (tmp102) Use devm_add_action to register  cleanup function Guenter Roeck <linux@roeck-us.net> - 2016-06-25 03:30 +0200

csiph-web