From: Lee Jones <lee.jones@linaro.org>
To: Jean Delvare <jdelvare@suse.de>
Cc: Laszlo Papp <lpapp@kde.org>,
linux-kernel@vger.kernel.org, Guenter Roeck <linux@roeck-us.net>,
lm-sensors@lm-sensors.org
Subject: Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
Date: Mon, 10 Feb 2014 16:58:53 +0000 [thread overview]
Message-ID: <20140210165853.GD26997@lee--X1> (raw)
In-Reply-To: <20140210173811.04ba5964@endymion.delvare>
PiA+ID4gIHN0YXRpYyBjb25zdCBzdHJ1Y3QgaTJjX2RldmljZV9pZCBtYXg2NjUwX2lkW10gPSB7
Cj4gPiA+IC0JeyAibWF4NjY1MCIsIDEgfSwKPiA+ID4gLQl7ICJtYXg2NjUxIiwgNCB9LAo+ID4g
PiArCXsgIm1heDY2NTAtaHdtb24iLCAxIH0sCj4gPiA+ICsJeyAibWF4NjY1MS1od21vbiIsIDQg
fSwKPiAKPiBObywgdGhpcyBpcyBub3QgYWNjZXB0YWJsZSwgc29ycnkuIFRoaXMgd2lsbCBjaGFu
Z2UgdGhlIG5hbWUgb2YgdGhlCj4gaHdtb24gZGV2aWNlIGFzIHNlZW4gZnJvbSB1c2VyLXNwYWNl
LCBicmVha2luZyBhbnkgY29uZmlndXJhdGlvbiBmaWxlCj4gcmVmZXJyaW5nIHRvIGl0LiBBZGRp
dGlvbmFsbHksIGRhc2hlcyBhcmUgZXhwbGljaXRseSBmb3JiaWRkZW4gaW4gaHdtb24KPiBkZXZp
Y2UgbmFtZXMuIEFuZCBsYXN0bHkgdGhpcyB3aWxsIGJyZWFrIGFueSBleHBsaWNpdCBpbnN0YW50
aWF0aW9uIG9mCj4gdGhlc2VzIGRldmljZXMgKHdoaWNoIGlzIHRoZSBvbmx5IHdheSwgYXMgdGhl
IGRyaXZlciBkb2Vzbid0IHN1cHBvcnQKPiBkZXZpY2UgYXV0by1kZXRlY3Rpb24pLCBiZSBpdCBp
biB0aGUga2VybmVsIGl0c2VsZiBvciBmcm9tIHVzZXItc3BhY2UuCj4gCj4gVGhlIGNoYW5nZSBk
b2Vzbid0IG1ha2Ugc2Vuc2UgYW55d2F5LiBJZiB5b3UgbW92ZSB0byB0aGUgTUZEIGZyYW1ld29y
aywKPiB0aGUgY29yZSBkcml2ZXIgd2lsbCBiZSBhbiBJMkMgZHJpdmVyIGJpbmRpbmcgdG8gdGhl
IEkyQyBkZXZpY2UsIGFuZCBpdAo+IHdpbGwgc3Bhd24gdGhlIGxvZ2ljYWwgZGV2aWNlcywgcHJl
c3VtYWJseSBpbiB0aGUgZm9ybSBvZiBwbGF0Zm9ybQo+IGRldmljZXMuIFRoYXQncyB3aGF0IHRo
ZSBjdXJyZW50IG1heDY2NTAgZHJpdmVyIHdvdWxkIGhhdmUgdG8gYmluZCB0by4KPiBKdXN0IHJl
bmFtaW5nIHRoZSBkZXZpY2Ugd29uJ3Qgd29yaywgeW91IGFsc28gbmVlZCB0byBjaGFuZ2UgdGhl
IHR5cGUuCj4gCj4gSWYgeW91IHdhbnQgdG8gdHVybiB0aGlzIGludG8gYW4gTUZEIGRyaXZlciwg
SSBiZWxpZXZlIHlvdSBtdXN0IGZpcnN0Cj4gY29udmVydCB0aGUgaHdtb24gcGFydCB0byByZWdp
c3RlciB1c2luZwo+IGRldm1faHdtb25fZGV2aWNlX3JlZ2lzdGVyX3dpdGhfZ3JvdXBzKCkuIFRo
aXMgd2lsbCBkaXNzb2NpYXRlIHRoZSBpMmMKPiBkZXZpY2UgbmFtZSBmcm9tIHRoZSBod21vbiBk
ZXZpY2UgbmFtZSBhbmQgY3JlYXRlIGEgY2xlYW4gbmFtZS1zcGFjZQo+IGZvciBlYWNoIGZ1bmN0
aW9uLiBHdWVudGVyLCBtYXliZSB5b3UgaGFkIGEgcGxhbiB0byBkbyBzbyBhbHJlYWR5Cj4gYW55
d2F5Pwo+IAo+IFRoYXQgYmVpbmcgc2FpZCwgZ29pbmcgd2l0aCBNRkQgaW4gdGhpcyBjYXNlIHNl
ZW1zIHF1aXRlIG92ZXJraWxsIHRvCj4gbWUuIE1GRCBtYWtlcyBhIGxvdCBvZiBzZW5zZSB3aGVu
IGVhY2ggZnVuY3Rpb24gaGFzIGl0cyBvd24gcmVzb3VyY2VzLgo+IEFzIHRoaXMgaXNuJ3QgdGhl
IGNhc2UgaGVyZSwgYSBzaW5nbGUgZHJpdmVyIHJlZ2lzdGVyaW5nIGJvdGggYW4gaHdtb24KPiBp
bnRlcmZhY2UgYW5kIGEgcGluY3RybCBpbnRlcmZhY2Ugd291bGQgc2VlbSBzdWZmaWNpZW50IHRv
IG1lLiBCdXQgSQo+IHRoaW5rIEd1ZW50ZXIgYWxyZWFkeSBkaXNjdXNzZWQgdGhpcyBpbiB0aGUg
cGFzdCBzbyBJJ2xsIGxldCBoaW0KPiBjb250aW51ZSBhbmQgZGVjaWRlLgoKSSdsbCBnZXQgeW91
IGd1eXMgZGVjaWRlIGlmIHlvdSB3YW50IHRvIGdvIHRoZSBNRkQgcm91dGUgb3Igbm90LgoKRWl0
aGVyIGlzIG9rYXkgd2l0aCBtZSwgYnV0IGlmIHlvdSBkbyBkZWNpZGUgaW4gZmF2b3VyLCBhIG5h
bWUgY2hhbmdlCndpdGggdGhlIGRldmljZSB0eXBlIGFwcGVuZGVkIHdvdWxkIGJlIHByZWZlcnJl
ZC4gRWxzZSB0aGUgY29yZSBkZXZpY2UKd291bGQgaGF2ZSB0aGUgc2FtZSBuYW1lIGFzIGFsbCBv
ZiBpdHMgY2hpbGRyZW4gd2hpY2ggd291bGQgYmUgcXVpdGUKdW53b3JrYWJsZS4KCj4gPiBNaWdo
dCBiZSB3b3J0aCB0YWtpbmcgdGhlIG9wcG9ydHVuaXR5IHRvIHN3YXAgb3V0IHRoZXNlIG1hZ2lj
IG51bWJlcnMKPiA+IG5vdy4KPiAKPiBUaGVyZSdzIG5vdGhpbmcgbWFnaWMgYWJvdXQgdGhlbSwg
dGhleSB0ZWxsIHRoZSBkcml2ZXIgaG93IG1hbnkgZmFucwo+IGVhY2ggZGV2aWNlIHN1cHBvcnRz
LiBJZiB5b3UgZG9uJ3QgcGFzcyB0aGVtIGFzIGRyaXZlcl9kYXRhIHlvdSdsbCBoYXZlCj4gdG8g
ZGVyaXZlIHRoZW0gZnJvbSB0aGUgZGV2aWNlIG5hbWUgaW4gdGhlIHByb2JlIGZ1bmN0aW9uLgoK
VGhleSdyZSBtYWdpYyBpbiB0aGF0IHRoZXkncmUgbm90IGVhc2lseSBpZGVudGlmaWFibGUuIElu
IHRoZSBmZXcKbW9tZW50cyB0aGF0IEkgbG9va2VkIGF0IHRoZSBwYXRjaCBJIGFzc3VtZWQgdGhl
eSB3ZXJlIGRldmljZQpJRHMuIFRoZXkgc2hvdWxkIGJlIGNsZWFybHkgZGVmaW5lZC4KCj4gPiA+
ICAJeyB9Cj4gPiA+ICB9Owo+ID4gPiAgTU9EVUxFX0RFVklDRV9UQUJMRShpMmMsIG1heDY2NTBf
aWQpOwoKLS0gCkxlZSBKb25lcwpMaW5hcm8gU1RNaWNyb2VsZWN0cm9uaWNzIExhbmRpbmcgVGVh
bSBMZWFkCkxpbmFyby5vcmcg4pSCIE9wZW4gc291cmNlIHNvZnR3YXJlIGZvciBBUk0gU29DcwpG
b2xsb3cgTGluYXJvOiBGYWNlYm9vayB8IFR3aXR0ZXIgfCBCbG9nCgpfX19fX19fX19fX19fX19f
X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpsbS1zZW5zb3JzIG1haWxpbmcgbGlzdAps
bS1zZW5zb3JzQGxtLXNlbnNvcnMub3JnCmh0dHA6Ly9saXN0cy5sbS1zZW5zb3JzLm9yZy9tYWls
bWFuL2xpc3RpbmZvL2xtLXNlbnNvcnM
WARNING: multiple messages have this Message-ID (diff)
From: Lee Jones <lee.jones@linaro.org>
To: Jean Delvare <jdelvare@suse.de>
Cc: Laszlo Papp <lpapp@kde.org>,
linux-kernel@vger.kernel.org, Guenter Roeck <linux@roeck-us.net>,
lm-sensors@lm-sensors.org
Subject: Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
Date: Mon, 10 Feb 2014 16:58:53 +0000 [thread overview]
Message-ID: <20140210165853.GD26997@lee--X1> (raw)
In-Reply-To: <20140210173811.04ba5964@endymion.delvare>
> > > static const struct i2c_device_id max6650_id[] = {
> > > - { "max6650", 1 },
> > > - { "max6651", 4 },
> > > + { "max6650-hwmon", 1 },
> > > + { "max6651-hwmon", 4 },
>
> No, this is not acceptable, sorry. This will change the name of the
> hwmon device as seen from user-space, breaking any configuration file
> referring to it. Additionally, dashes are explicitly forbidden in hwmon
> device names. And lastly this will break any explicit instantiation of
> theses devices (which is the only way, as the driver doesn't support
> device auto-detection), be it in the kernel itself or from user-space.
>
> The change doesn't make sense anyway. If you move to the MFD framework,
> the core driver will be an I2C driver binding to the I2C device, and it
> will spawn the logical devices, presumably in the form of platform
> devices. That's what the current max6650 driver would have to bind to.
> Just renaming the device won't work, you also need to change the type.
>
> If you want to turn this into an MFD driver, I believe you must first
> convert the hwmon part to register using
> devm_hwmon_device_register_with_groups(). This will dissociate the i2c
> device name from the hwmon device name and create a clean name-space
> for each function. Guenter, maybe you had a plan to do so already
> anyway?
>
> That being said, going with MFD in this case seems quite overkill to
> me. MFD makes a lot of sense when each function has its own resources.
> As this isn't the case here, a single driver registering both an hwmon
> interface and a pinctrl interface would seem sufficient to me. But I
> think Guenter already discussed this in the past so I'll let him
> continue and decide.
I'll get you guys decide if you want to go the MFD route or not.
Either is okay with me, but if you do decide in favour, a name change
with the device type appended would be preferred. Else the core device
would have the same name as all of its children which would be quite
unworkable.
> > Might be worth taking the opportunity to swap out these magic numbers
> > now.
>
> There's nothing magic about them, they tell the driver how many fans
> each device supports. If you don't pass them as driver_data you'll have
> to derive them from the device name in the probe function.
They're magic in that they're not easily identifiable. In the few
moments that I looked at the patch I assumed they were device
IDs. They should be clearly defined.
> > > { }
> > > };
> > > MODULE_DEVICE_TABLE(i2c, max6650_id);
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
next prev parent reply other threads:[~2014-02-10 16:58 UTC|newest]
Thread overview: 64+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-10 15:25 [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix Laszlo Papp
2014-02-10 15:25 ` Laszlo Papp
2014-02-10 16:08 ` [lm-sensors] " Lee Jones
2014-02-10 16:08 ` Lee Jones
2014-02-10 16:38 ` [lm-sensors] " Jean Delvare
2014-02-10 16:38 ` Jean Delvare
2014-02-10 16:53 ` linux
2014-02-10 16:53 ` linux
2014-02-10 18:59 ` Laszlo Papp
2014-02-10 18:59 ` Laszlo Papp
2014-02-10 23:10 ` Guenter Roeck
2014-02-10 23:10 ` Guenter Roeck
2014-02-11 3:23 ` Laszlo Papp
2014-02-11 3:23 ` Laszlo Papp
2014-02-11 3:35 ` Laszlo Papp
2014-02-11 3:35 ` Laszlo Papp
2014-02-10 16:58 ` Lee Jones [this message]
2014-02-10 16:58 ` Lee Jones
2014-02-10 17:43 ` Jean Delvare
2014-02-10 17:43 ` Jean Delvare
2014-02-10 18:01 ` [lm-sensors] " Lee Jones
2014-02-10 18:01 ` Lee Jones
2014-02-10 18:15 ` [lm-sensors] " Jean Delvare
2014-02-10 18:15 ` Jean Delvare
2014-02-10 18:24 ` Lee Jones
2014-02-10 18:24 ` Lee Jones
2014-02-10 18:27 ` Laszlo Papp
2014-02-10 18:27 ` Laszlo Papp
2014-02-10 18:55 ` [lm-sensors] " Jean Delvare
2014-02-10 18:55 ` Jean Delvare
2014-02-10 17:06 ` Laszlo Papp
2014-02-10 17:06 ` Laszlo Papp
2014-02-10 17:09 ` Laszlo Papp
2014-02-10 17:09 ` Laszlo Papp
2014-02-11 3:13 ` Laszlo Papp
2014-02-11 3:13 ` Laszlo Papp
2014-02-11 7:50 ` Jean Delvare
2014-02-11 7:50 ` Jean Delvare
2014-02-11 8:19 ` Laszlo Papp
2014-02-11 8:19 ` Laszlo Papp
2014-02-11 8:28 ` Laszlo Papp
2014-02-11 8:28 ` Laszlo Papp
2014-02-11 8:49 ` Jean Delvare
2014-02-11 8:49 ` Jean Delvare
2014-02-11 9:08 ` Laszlo Papp
2014-02-11 9:08 ` Laszlo Papp
2014-02-11 9:57 ` Lee Jones
2014-02-11 9:57 ` Lee Jones
2014-02-11 15:15 ` Laszlo Papp
2014-02-11 15:15 ` Laszlo Papp
2014-02-11 8:50 ` Lee Jones
2014-02-11 8:50 ` Lee Jones
2014-02-11 8:58 ` Laszlo Papp
2014-02-11 8:58 ` Laszlo Papp
2014-02-11 9:14 ` Laszlo Papp
2014-02-11 9:14 ` Laszlo Papp
2014-02-11 9:47 ` Lee Jones
2014-02-11 9:47 ` Lee Jones
2014-02-11 9:50 ` Laszlo Papp
2014-02-11 9:50 ` Laszlo Papp
2014-02-11 10:22 ` Laszlo Papp
2014-02-11 10:22 ` Laszlo Papp
2014-02-11 11:09 ` Laszlo Papp
2014-02-11 11:09 ` Laszlo Papp
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=20140210165853.GD26997@lee--X1 \
--to=lee.jones@linaro.org \
--cc=jdelvare@suse.de \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=lm-sensors@lm-sensors.org \
--cc=lpapp@kde.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.