Re: [PATCH] spi: Master driver for NXP SC18IS602/603

[Guenter Roeck <linux@roeck-us.net>]

On Wed, Aug 22, 2012 at 07:32:30PM +0100, Mark Brown wrote:
> On Sat, Aug 18, 2012 at 09:06:27AM -0700, Guenter Roeck wrote:
> > This driver adds support for NXP SC18IS602/603 I2C to SPI bus bridge.
> > 
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> 
> Applied, thanks.  One small thing:
> 
> > +static int sc18is602_probe(struct i2c_client *client,
> > +			   const struct i2c_device_id *id)
> > +{
> > +	struct device *dev = &client->dev;
> > +	struct device_node *np = dev->of_node;
> > +	struct sc18is602_platform_data *pdata = dev_get_platdata(dev);
> > +	struct sc18is602 *hw;
> > +	struct spi_master *master;
> > +	int error;
> > +
> > +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C |
> > +				     I2C_FUNC_SMBUS_WRITE_BYTE_DATA))
> > +		return -ENODEV;
> 
> This feels like the wrong thing to do here: given that the user needs to
> explicitly ask for the device to be instantiated we really ought to be
> one the right bus so this should be more of an error.  Not sure what I
> can constructively sugget except for an error message and perhaps
> returning -EINVAL though.

Hi Mark,

Makes sense. Changing the return value to -EINVAL results in the following
warning message from the driver subsystem.

sc18is602: probe of 7-0010 failed with error -22

I think this should be good enough, as there are other possible reasons for the
call to fail which result in similar messages. Do you want to make that change,
or should I change it and resubmit ?

Thanks,
Guenter