On Wed, Dec 3, 2008 at 9:01 PM, Felipe Balbi <felipe.balbi@xxxxxxxxx> wrote:
...
>> > +
>> > +static int tsc2007_xfer(void *tsc, unsigned char cmd)
>> > +{
>> > + struct tsc2007 *ts = tsc;
>> > + struct i2c_client *client = ts->client;
>> > +
>> > + unsigned char rbuf[2];
>> > + unsigned short val;
>> > + int result;
>> > +
>> > + result = i2c_master_send(client, &cmd, 1);
>> > + if (result != 1) {
>> > + dev_err(&client->dev, "send failed, cmd 0x%x\n", cmd);
>> > + goto cmd_fail;
>> > + }
>> > +
>> > + result = i2c_master_recv(client, rbuf, 2);
>> > + if (result != 2) {
>> > + dev_err(&client->dev, "recv failed, cmd 0x%x\n", cmd);
>> > + goto cmd_fail;
>> > + }
>> > +
>> > + rbuf[1] = (rbuf[1] >> 4) | ((rbuf[0] & 0x0f) << 4);
>> > + rbuf[0] = rbuf[0] >> 4;
>> > +
>> > + val = *((unsigned short *) rbuf);
>> > + val = be16_to_cpu(val);
>> > +
>> > + dev_dbg(&client->dev, "cmd [0x%x], rbuf [0x%x, 0x%x] => val [%u]\n",
>> > + cmd, rbuf[0], rbuf[1], val);
>> > +
>> > + return (int) val;
>> > +
>> > +cmd_fail:
>> > + return -EIO;
>> > +}
>
> so you really need to go that low level ?? Did you try
> i2c_smbus_write_*() ??
>
Because the platform which I use did not implement smbus_xfer yet,
it only implemented master_xfer in struct i2c_algorithm.
I can add smbus_xfer for my platform. I have one question and
I know that smbus is a subset from the I2C protocol.
Is smbus_xfer method better than master_xfer or is there any
special reason to use smbus_xfer?
>> > +
>> > +static int tsc2007_probe(struct i2c_client *client,
>> > + const struct i2c_device_id *id)
>> > +{
>> > + struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
>
> why ??
It's for i2c_check_functionality(). This function requried struct
i2c_adapter as an argument.
>> > + struct tsc2007 *ts;
>> > + struct tsc2007_platform_data *pdata;
>> > + struct input_dev *input_dev;
>> > + int err;
>> > +
>> > + pdata = client->dev.platform_data;
>> > + if (pdata == NULL) {
>> > + dev_err(&client->dev, "platform data is required!\n");
>> > + return -EINVAL;
>> > + }
>> > +
>> > + if (!i2c_check_functionality(adapter,
>> > + I2C_FUNC_I2C | I2C_FUNC_SMBUS_WRITE_BYTE_DATA))
>> > + return -EIO;
>
> should not be in this driver, I'd say.
I saw that some I2C driver uses this function, for example - max6875.c.
Is i2c_check_functionality() not necessary for a I2C client driver?
>> > + ts->irq = client->irq;
>> > + if (request_irq(ts->irq, tsc2007_irq, 0,
>> > + client->dev.driver->name, ts)) {
>> > + dev_err(&client->dev, "irq %d busy?\n", ts->irq);
>> > + err = -EBUSY;
>> > + goto err_free_mem;
>
> would be better to:
>
> err = request_irq(...)
> if (err < 0) {
> dev_err(..)
> ...
> }
>
> Then we see the error request_irq() returned, which is more useful.
Thank you very much for your kind comments.
> Would be nice to get comments from Jean Delvare and Ben Dooks as well.
I also want to get more comments. Thank you, again.
--
Kwangwoo Lee <kwangwoo.lee@xxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
