All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
To: Anders Berg <anders.berg-1wcpHE2jlwO1Z/+hSey0Gg@public.gmane.org>
Cc: devicetree <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
	<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>,
	"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH] i2c: axxia: Add I2C driver for AXM55xx
Date: Mon, 22 Sep 2014 13:04:55 +0200	[thread overview]
Message-ID: <20140922110455.GA1463@katana> (raw)
In-Reply-To: <CAE0=X_1v_1eNr=dPTgofamFuy6zP9ff=LS7dyexj4CeutDqaPA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 1164 bytes --]


> >> >> +     if (msg->len == 0 || msg->len > 255)
> >> >> +             return -EINVAL;
> >> >
> >> > Ouch, really? Maybe we should warn the user here.
> >>
> >> Yeah, the transfer length register limits the length to 255. I'll add
> >> a warning here.
> >
> > Please also add this information to the Kconfig description and
> > somewhere at the top of the source file. This is an important flaw which
> > people should easily find out about.
> >
> 
> You are referring to the "len <= 255" restriction being the flaw here,
> right? I'll add a note to Kconfig and the driver about that.

Yes, I meant that. I just remembered we should do something else:

Remove I2C_FUNC_I2C (because it cannot do endless transfers) from
functionality and simply use I2C_FUNC_SMBUS_I2C_BLOCK which does I2C
like transfers with the SMBus Limit of 32 bytes. It seems PMBus allows
for 255 byte which this HW could support, yet I don't recall we have
support for that size currently.

> The other part of the condition (msg->len == 0) should actually go
> away. The controller can do zero-length-data transfers. I'll fix that
> for next round.

Great!


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: wsa@the-dreams.de (Wolfram Sang)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] i2c: axxia: Add I2C driver for AXM55xx
Date: Mon, 22 Sep 2014 13:04:55 +0200	[thread overview]
Message-ID: <20140922110455.GA1463@katana> (raw)
In-Reply-To: <CAE0=X_1v_1eNr=dPTgofamFuy6zP9ff=LS7dyexj4CeutDqaPA@mail.gmail.com>


> >> >> +     if (msg->len == 0 || msg->len > 255)
> >> >> +             return -EINVAL;
> >> >
> >> > Ouch, really? Maybe we should warn the user here.
> >>
> >> Yeah, the transfer length register limits the length to 255. I'll add
> >> a warning here.
> >
> > Please also add this information to the Kconfig description and
> > somewhere at the top of the source file. This is an important flaw which
> > people should easily find out about.
> >
> 
> You are referring to the "len <= 255" restriction being the flaw here,
> right? I'll add a note to Kconfig and the driver about that.

Yes, I meant that. I just remembered we should do something else:

Remove I2C_FUNC_I2C (because it cannot do endless transfers) from
functionality and simply use I2C_FUNC_SMBUS_I2C_BLOCK which does I2C
like transfers with the SMBus Limit of 32 bytes. It seems PMBus allows
for 255 byte which this HW could support, yet I don't recall we have
support for that size currently.

> The other part of the condition (msg->len == 0) should actually go
> away. The controller can do zero-length-data transfers. I'll fix that
> for next round.

Great!

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140922/1e878942/attachment.sig>

WARNING: multiple messages have this Message-ID (diff)
From: Wolfram Sang <wsa@the-dreams.de>
To: Anders Berg <anders.berg@avagotech.com>
Cc: devicetree <devicetree@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	linux-i2c@vger.kernel.org
Subject: Re: [PATCH] i2c: axxia: Add I2C driver for AXM55xx
Date: Mon, 22 Sep 2014 13:04:55 +0200	[thread overview]
Message-ID: <20140922110455.GA1463@katana> (raw)
In-Reply-To: <CAE0=X_1v_1eNr=dPTgofamFuy6zP9ff=LS7dyexj4CeutDqaPA@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1164 bytes --]


> >> >> +     if (msg->len == 0 || msg->len > 255)
> >> >> +             return -EINVAL;
> >> >
> >> > Ouch, really? Maybe we should warn the user here.
> >>
> >> Yeah, the transfer length register limits the length to 255. I'll add
> >> a warning here.
> >
> > Please also add this information to the Kconfig description and
> > somewhere at the top of the source file. This is an important flaw which
> > people should easily find out about.
> >
> 
> You are referring to the "len <= 255" restriction being the flaw here,
> right? I'll add a note to Kconfig and the driver about that.

Yes, I meant that. I just remembered we should do something else:

Remove I2C_FUNC_I2C (because it cannot do endless transfers) from
functionality and simply use I2C_FUNC_SMBUS_I2C_BLOCK which does I2C
like transfers with the SMBus Limit of 32 bytes. It seems PMBus allows
for 255 byte which this HW could support, yet I don't recall we have
support for that size currently.

> The other part of the condition (msg->len == 0) should actually go
> away. The controller can do zero-length-data transfers. I'll fix that
> for next round.

Great!


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

  parent reply	other threads:[~2014-09-22 11:04 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-25 11:51 [PATCH] i2c: axxia: Add I2C driver for AXM55xx Anders Berg
2014-08-25 11:51 ` Anders Berg
2014-08-25 11:51 ` Anders Berg
     [not found] ` <1408967482-17723-1-git-send-email-anders.berg-1wcpHE2jlwO1Z/+hSey0Gg@public.gmane.org>
2014-09-20 12:12   ` Wolfram Sang
2014-09-20 12:12     ` Wolfram Sang
2014-09-20 12:12     ` Wolfram Sang
2014-09-22  9:19     ` Anders Berg
2014-09-22  9:19       ` Anders Berg
2014-09-22  9:19       ` Anders Berg
     [not found]       ` <CAE0=X_0VQ8rWrUgbsgKm1MwCP8bwWm3UJPq9p=xNDfjP4CC7Bg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-09-22  9:59         ` Wolfram Sang
2014-09-22  9:59           ` Wolfram Sang
2014-09-22  9:59           ` Wolfram Sang
2014-09-22 10:24           ` Anders Berg
2014-09-22 10:24             ` Anders Berg
2014-09-22 10:24             ` Anders Berg
2014-09-22 10:47           ` Anders Berg
2014-09-22 10:47             ` Anders Berg
     [not found]             ` <CAE0=X_1v_1eNr=dPTgofamFuy6zP9ff=LS7dyexj4CeutDqaPA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-09-22 11:04               ` Wolfram Sang [this message]
2014-09-22 11:04                 ` Wolfram Sang
2014-09-22 11:04                 ` Wolfram Sang
2014-09-22 12:18                 ` Anders Berg
2014-09-22 12:18                   ` Anders Berg
2014-09-22 13:03           ` Russell King - ARM Linux
2014-09-22 13:03             ` Russell King - ARM Linux
2014-09-22 13:03             ` Russell King - ARM Linux
     [not found]             ` <20140922130351.GL5182-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>
2014-09-22 14:49               ` Wolfram Sang
2014-09-22 14:49                 ` Wolfram Sang
2014-09-22 14:49                 ` Wolfram Sang
2014-09-22 13:16     ` Uwe Kleine-König
2014-09-22 13:16       ` Uwe Kleine-König
2014-09-22 13:16       ` Uwe Kleine-König

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=20140922110455.GA1463@katana \
    --to=wsa-z923lk4zbo2bacvfa/9k2g@public.gmane.org \
    --cc=anders.berg-1wcpHE2jlwO1Z/+hSey0Gg@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.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.