All of lore.kernel.org
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Nishanth Menon <nm@ti.com>
Cc: Jean Delvare <jdelvare@suse.com>,
	linux-hwmon@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/5] hwmon: (tmp102) Use devm_add_action to register cleanup function
Date: Fri, 24 Jun 2016 09:36:04 -0700	[thread overview]
Message-ID: <20160624163604.GA21447@roeck-us.net> (raw)
In-Reply-To: <576D505E.3020308@ti.com>

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

  parent reply	other threads:[~2016-06-24 16:36 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-24  0:28 Guenter Roeck
2016-06-24  0:28 ` [PATCH 2/5] hwmon: (tmp102) Drop FSF address Guenter Roeck
2016-06-24  0:28 ` [PATCH 3/5] hwmon: (tmp102) Improve handling of initial read delay Guenter Roeck
2016-06-24  0:28 ` [PATCH 4/5] hwmon: (tmp102) Rework chip configuration Guenter Roeck
2016-06-24  0:28 ` [PATCH 5/5] hwmon: (tmp102) Convert to use regmap, and drop local cache Guenter Roeck
2016-06-24 14:13 ` [PATCH 1/5] hwmon: (tmp102) Use devm_add_action to register cleanup function Nishanth Menon
2016-06-24 14:30   ` Guenter Roeck
2016-06-24 14:54     ` Guenter Roeck
2016-06-24 15:23       ` Nishanth Menon
2016-06-24 15:31         ` Nishanth Menon
2016-06-24 16:36         ` Guenter Roeck [this message]
2016-06-24 18:02           ` Nishanth Menon
2016-06-24 18:18             ` Guenter Roeck
2016-06-24 18:21               ` Nishanth Menon
2016-06-25  1:24                 ` Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160624163604.GA21447@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=jdelvare@suse.com \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nm@ti.com \
    --subject='Re: [PATCH 1/5] hwmon: (tmp102) Use devm_add_action to register cleanup function' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.