RE: [PATCH v4 05/12] [media] cxd2880: Add tuner part of the driver

[Takiguchi, Yasunari]

Hi, Mauro


> > +	ret = tnr_dmd->io->read_regs(tnr_dmd->io,
> > +				     CXD2880_IO_TGT_SYS,
> > +				     0x10, data, 1);
> > +	if (ret)
> > +		return ret;
> > +	if ((data[0] & 0x01) == 0x00)
> > +		return -EBUSY;
> 
> I don't know anything about this hardware, but it sounds weird to return
> -EBUSY here, except if the hardware reached a permanent busy condition,
> and would require some sort of reset to work again.
> 
> As this is in the middle of lots of things, I *suspect* that this is
> not the case.
> 
> If I'm right, and this is just a transitory solution that could happen
> for a limited amount of time, e. g. if what's there at data[0] is a flag
> saying that the device didn't finish the last operation yet, maybe the
> best would be to do something like:
> 
> 	for (i = 0; i < 10; i++) {
> 		ret = tnr_dmd->io->read_regs(tnr_dmd->io,
> 					     CXD2880_IO_TGT_SYS,
> 					     0x10, data, 1);
> 		if (ret)
> 			return ret;
> 		if (data[0] & 0x01)
> 			break;
> 		msleep(10);
> 	}
> 	if (!(data[0] & 0x01))
> 		return -EBUSY;
> 
> > +
> > +	ret = cxd2880_io_write_multi_regs(tnr_dmd->io,
> > +					  CXD2880_IO_TGT_SYS,
> > +					  rf_init1_seq5,
> > +					  ARRAY_SIZE(rf_init1_seq5));
> > +	if (ret)
> > +		return ret;
> > +
> > +	usleep_range(1000, 2000);
> > +
> > +	ret = tnr_dmd->io->write_reg(tnr_dmd->io,
> > +				     CXD2880_IO_TGT_SYS,
> > +				     0x00, 0x0a);
> > +	if (ret)
> > +		return ret;
> > +	ret = tnr_dmd->io->read_regs(tnr_dmd->io,
> > +				     CXD2880_IO_TGT_SYS,
> > +				     0x11, data, 1);
> > +	if (ret)
> > +		return ret;
> > +	if ((data[0] & 0x01) == 0x00)
> > +		return -EBUSY;
> 
> Same here and on similar places.

As the hardware specification, It is abnormal if certain register doesn't become 1 even if sleep time passes.
Perhaps it should not be return EBUSY.
We will reconsider error code.

Thanks,
Takiguchi