netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Sergio Paracuellos <sergio.paracuellos@gmail.com>
Cc: Tristram.Ha@microchip.com,
	Florian Fainelli <f.fainelli@gmail.com>,
	Pavel Machek <pavel@ucw.cz>, Marek Vasut <marex@denx.de>,
	Dan Carpenter <dan.carpenter@oracle.com>,
	vivien.didelot@savoirfairelinux.com,
	UNGLinuxDriver@microchip.com, netdev@vger.kernel.org
Subject: Re: [PATCH RFC net-next] net: dsa: microchip: add KSZ9477 I2C driver
Date: Tue, 18 Dec 2018 11:01:32 +0100	[thread overview]
Message-ID: <20181218100132.GE8334@lunn.ch> (raw)
In-Reply-To: <CAMhs-H-rP9jGvDArGyx4u+YL8q9Kku2kmefCc9D31Q5adAZ4fw@mail.gmail.com>

On Tue, Dec 18, 2018 at 07:24:52AM +0100, Sergio Paracuellos wrote:
> > +static int ksz9477_i2c_read_reg(struct i2c_client *i2c, u32 reg, u8 *val,
> > +                               unsigned int len)
> > +{
> > +       struct i2c_msg msg[2];
> > +       int ret = 0;
> > +
> > +       val[0] = (u8)(reg >> 8);
> > +       val[1] = (u8)reg;
> > +
> > +       msg[0].addr = i2c->addr;
> > +       msg[0].flags = 0;
> > +       msg[0].len = 2;
> > +       msg[0].buf = val;
> > +
> > +       msg[1].addr = i2c->addr;
> > +       msg[1].flags = I2C_M_RD;
> > +       msg[1].len = len;
> > +       msg[1].buf = &val[2];
> > +
> > +       if (i2c_transfer(i2c->adapter, msg, 2) != 2)
> > +               ret = -ENODEV;
> 
> Should this be -EREMOTEIO instead?

It should actually be whatever error code i2c_transfer returns.

> > +static int ksz9477_i2c_write_reg(struct i2c_client *i2c, u32 reg, u8 *val,
> > +                                unsigned int len)
> > +{
> > +       struct i2c_msg msg;
> > +       int ret = 0;
> > +
> > +       val[0] = (u8)(reg >> 8);
> > +       val[1] = (u8)reg;
> > +
> > +       msg.addr = i2c->addr;
> > +       msg.flags = 0;
> > +       msg.len = 2 + len;
> > +       msg.buf = val;
> > +
> > +       if (i2c_transfer(i2c->adapter, &msg, 1) != 1)
> > +               ret = -ENODEV;
> 
> 
> Should this be -EREMOTEIO instead?

And the same here.

> > +MODULE_AUTHOR("Woojung Huh <Woojung.Huh@microchip.com>");
> > +MODULE_DESCRIPTION("Microchip KSZ9477 Series Switch I2C access Driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/net/dsa/microchip/ksz_i2c.h b/drivers/net/dsa/microchip/ksz_i2c.h
> > new file mode 100644
> > index 0000000..b9af0a8
> > --- /dev/null
> > +++ b/drivers/net/dsa/microchip/ksz_i2c.h
> > @@ -0,0 +1,69 @@
> > +/* SPDX-License-Identifier: GPL-2.0

This is i think inconsistent. MODULE_LICENSE("GPL") means GPL 2 and at
your choice, future versions. Your SPDX is for v2 only.

     Andrew

  reply	other threads:[~2018-12-18 10:01 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-18  1:39 [PATCH RFC net-next] net: dsa: microchip: add KSZ9477 I2C driver Tristram.Ha
2018-12-18  1:40 ` Tristram.Ha
2018-12-18  2:55   ` Woojung.Huh
2018-12-18  4:16   ` Tristram.Ha
2018-12-18  6:30     ` Sergio Paracuellos
2018-12-18  6:24   ` Sergio Paracuellos
2018-12-18 10:01     ` Andrew Lunn [this message]
2018-12-18 14:49       ` Marek Vasut
2018-12-18  8:06 ` Dan Carpenter
2018-12-18  8:54   ` Sergio Paracuellos

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=20181218100132.GE8334@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=Tristram.Ha@microchip.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=dan.carpenter@oracle.com \
    --cc=f.fainelli@gmail.com \
    --cc=marex@denx.de \
    --cc=netdev@vger.kernel.org \
    --cc=pavel@ucw.cz \
    --cc=sergio.paracuellos@gmail.com \
    --cc=vivien.didelot@savoirfairelinux.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
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;
as well as URLs for NNTP newsgroup(s).