Linux-i3c Archive on lore.kernel.org
 help / color / Atom feed
From: Boris Brezillon <boris.brezillon@collabora.com>
To: vitor <vitor.soares@synopsys.com>
Cc: Przemyslaw Gaj <pgaj@cadence.com>,
	linux-i3c@lists.infradead.org, rafalc@cadence.com,
	bbrezillon@kernel.org
Subject: Re: [PATCH v2 1/3] i3c: Drop support for I2C 10 bit addresing
Date: Wed, 27 Feb 2019 13:10:04 +0100
Message-ID: <20190227131004.66ed9fa1@collabora.com> (raw)
In-Reply-To: <7607a23a-725d-b45a-a221-8a48bda301bd@synopsys.com>

Hi Vitor,

On Wed, 27 Feb 2019 12:05:30 +0000
vitor <vitor.soares@synopsys.com> wrote:

> On 26/02/19 14:28, Przemyslaw Gaj wrote:
> > This patch dropps support for I2C devices with 10 bit addressing. When I2C
> > device with 10 bit address is defined in DT, I3C master registration fails.
> >
> > Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com>
> >
> > ---
> >
> > Main changes between v1 and v2 are:
> > - Add error message when registering I2C device with 10 bit address.
> >
> > ---
> >  drivers/i3c/master.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> > index 2dc628d..8c1e365 100644
> > --- a/drivers/i3c/master.c
> > +++ b/drivers/i3c/master.c
> > @@ -1962,6 +1962,15 @@ of_i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master,
> >  	if (ret)
> >  		return ret;
> >  
> > +	/* The I3C Specification does not clearly say I2C devices with 10-bit
> > +	 * address are supported. These devices can't be passed properly through
> > +	 * DEFSLVS command.
> > +	 */  
> 
> IMO we just need to say 10-bit address not used or not supported in i3c.

I'd like to keep a clear comment on why it cannot supported right now
even though the spec is unclear about that. If the spec is updated to
state that I2C devs using extended addresses are forbidden, then we'll
update this comment accordingly.

> 
> > +	if (boardinfo->base.flags & I2C_CLIENT_TEN) {
> > +		dev_err(&master->dev, "I2C device with 10 bit address not supported.");
> > +		return -ENOTSUPP;
> > +	}
> > +
> >  	/* LVR is encoded in reg[2]. */
> >  	boardinfo->lvr = reg[2];
> >    
> 
> Also need to change:
> 
> #define I2C_MAX_ADDR            GENMASK(9, 0)
> to
> #define I2C_MAX_ADDR            GENMASK(6, 0)
> in master.h file

Yep, you can reduce the address space.

> 
> Not sure about:
> unsigned long addrslots[((I2C_MAX_ADDR + 1) * 2) / BITS_PER_LONG];
> @Boris can you check this part?

This part is still valid, no need to update it as you already updated
I2C_MAX_ADDR.

Thanks for the review.

Boris

_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

  reply index

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-26 14:28 [PATCH v2 0/3] Drop support for I2C 10 bit devices from I3C subsystem Przemyslaw Gaj
2019-02-26 14:28 ` [PATCH v2 1/3] i3c: Drop support for I2C 10 bit addresing Przemyslaw Gaj
2019-02-27 12:05   ` vitor
2019-02-27 12:10     ` Boris Brezillon [this message]
2019-02-27 12:52       ` vitor
2019-02-27 13:09         ` Boris Brezillon
2019-02-26 14:28 ` [PATCH v2 2/3] i3c: master: cdns: Drop support for I2C 10 bit addresing in Cadence I3C master Przemyslaw Gaj
2019-02-26 14:28 ` [PATCH v2 3/3] dt-bindings: i3c: Document dropped support for I2C 10 bit devices Przemyslaw Gaj

Reply instructions:

You may reply publically 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=20190227131004.66ed9fa1@collabora.com \
    --to=boris.brezillon@collabora.com \
    --cc=bbrezillon@kernel.org \
    --cc=linux-i3c@lists.infradead.org \
    --cc=pgaj@cadence.com \
    --cc=rafalc@cadence.com \
    --cc=vitor.soares@synopsys.com \
    /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

Linux-i3c Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-i3c/0 linux-i3c/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-i3c linux-i3c/ https://lore.kernel.org/linux-i3c \
		linux-i3c@lists.infradead.org linux-i3c@archiver.kernel.org
	public-inbox-index linux-i3c


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-i3c


AGPL code for this site: git clone https://public-inbox.org/ public-inbox