All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nishanth Menon <nm@ti.com>
To: Guenter Roeck <linux@roeck-us.net>, Jean Delvare <jdelvare@suse.com>
Cc: <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 10:31:24 -0500	[thread overview]
Message-ID: <576D524C.2030202@ti.com> (raw)
In-Reply-To: <576D505E.3020308@ti.com>

On 06/24/2016 10:23 AM, 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 
>>> 08: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> 10: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> 18: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> 20: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> 28: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> 30: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> 38: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> 40: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> 48: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> 50: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> 58: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> 60: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> 68: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> 70: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> 78: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> 80: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> 88: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> 90: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> 98: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> a0: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> a8: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> b0: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> b8: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> c0: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> c8: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> d0: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> d8: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> e0: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> e8: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> f0: 7912 b062 004b 0050 c018 e006 0000 0000 
>>> f8: 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 
>>> 08: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> 10: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> 18: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> 20: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> 28: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> 30: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> 38: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> 40: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> 48: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> 50: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> 58: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> 60: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> 68: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> 70: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> 78: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> 80: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> 88: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> 90: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> 98: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> a0: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> a8: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> b0: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> b8: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> c0: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> c8: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> d0: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> d8: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> e0: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> e8: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> f0: 5024 a060 004b 0050 c018 e006 0000 0000 
>>> f8: 5024 a060 004b 0050 c018 e006 0000 0000 
>>
> 
> 
> 
>> Also, beagleboard uses the omap i2c controller, correct ? I'll have to check
>> if regmap handles endianness conversion properly for controllers supporting
>> I2C_FUNC_I2C. If some other i2c controller is used, please let me know (or
>> sene me the output of i2cdetect -l). Recent kernels have a bug in regmap
>> which affects chips with 16-bit registers if I2C_FUNC_I2C is not supported.
Forgot to send you -l output:
root@BeagleBoard-X15:~# i2cdetect -l
i2c-0   i2c             OMAP I2C adapter                        I2C
adapter
i2c-2   i2c             OMAP I2C adapter                        I2C
adapter
root@BeagleBoard-X15:~# i2cdetect -F 0
Functionalities implemented by /dev/i2c-0:
I2C                              yes
SMBus Quick Command              no
SMBus Send Byte                  yes
SMBus Receive Byte               yes
SMBus Write Byte                 yes
SMBus Read Byte                  yes
SMBus Write Word                 yes
SMBus Read Word                  yes
SMBus Process Call               yes
SMBus Block Write                yes
SMBus Block Read                 no
SMBus Block Process Call         no
SMBus PEC                        yes
I2C Block Write                  yes
I2C Block Read                   yes


>>
>> Maybe I should get a beagleboard. The X15 isn't available yet, though. Do the
>> available boards (eg Rev C) use the same i2c controller ?
> 
> yeah there are variants of omap-i2c controllers, but basically they
> work in the same way.
> 
> I can try and debug the series once I get some spare time, might be
> over the weekend or next week.
> 


-- 
Regards,
Nishanth Menon

  reply	other threads:[~2016-06-24 15:32 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 [this message]
2016-06-24 16:36         ` Guenter Roeck
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=576D524C.2030202@ti.com \
    --to=nm@ti.com \
    --cc=jdelvare@suse.com \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --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.