public inbox for linux-hwmon@vger.kernel.org
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: "thilo.cestonaro@ts.fujitsu.com" <thilo.cestonaro@ts.fujitsu.com>,
	"linux-hwmon@vger.kernel.org" <linux-hwmon@vger.kernel.org>
Cc: "thilo@cestona.ro" <thilo@cestona.ro>
Subject: Re: [PATCH] added kernel module for FTS sensor chip "Teutates"
Date: Thu, 9 Jun 2016 06:25:18 -0700	[thread overview]
Message-ID: <57596E3E.30705@roeck-us.net> (raw)
In-Reply-To: <1465463437.4558.6.camel@ts.fujitsu.com>

On 06/09/2016 02:13 AM, thilo.cestonaro@ts.fujitsu.com wrote:
> Hey Guenter,
>
>>> +static int ftsteutates_detect(struct i2c_client *client,
>>> +		struct i2c_board_info *info)
>>> +{
>>> +	int val = ftsteutates_read_byte(client, FTSTEUTATES_DEVICE_ID_REG);
>>> +
>>> +	/* Baseboard Management Controller */
>>> +	if ((val & 0xF0) == 0x10) {
>>> +		switch (val & 0x0F) {
>>> +		case 0x01:
>>> +			strlcpy(info->type, ftsteutates_id[teutates].name,
>>> +			I2C_NAME_SIZE);
>>> +			info->flags = 0;
>>> +			return 0;
>>> +		}
>>> +	}
>> This is not sufficient for a detect function; it would result in many false
>> positives. Please drop it unless a much better means to detect the chip
>> is available.
>>
> Would it be sufficient, if I use dmi_name_in_vendors and check for Fujitsu? So the
> BMC check will only be used on FJ Boards?
>
> The problem is, that the firmware itself has only this device ID register.
> For future revisions I'll ask for more possibilities to identify the chip but for now
> I need to use other methods.
>

You don't need a detect function to start with. Using DMI in an i2c driver
is not appropriate either; an i2c driver is expected to be board agnostic.

If you can determine that the chip is present in the system using DMI, you
would normally have a second (platform)) driver which detects the platform
and instantiate the device, using either i2c_new_device() or
i2c_register_board_info(). See  Documentation/i2c/instantiating-devices
for more details.

Hope this helps,
Guenter


      reply	other threads:[~2016-06-09 13:25 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-31  8:55 [PATCH] added kernel module for FTS sensor chip "Teutates" Thilo Cestonaro
2016-06-04  5:51 ` Guenter Roeck
2016-06-06  9:21   ` thilo.cestonaro
2016-06-07  3:18     ` Guenter Roeck
2016-06-07  7:39       ` thilo.cestonaro
2016-06-07 13:30         ` Guenter Roeck
2016-06-09  9:13   ` thilo.cestonaro
2016-06-09 13:25     ` Guenter Roeck [this message]

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=57596E3E.30705@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=thilo.cestonaro@ts.fujitsu.com \
    --cc=thilo@cestona.ro \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox