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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 20 Dec 2017 01:01:59 +0000
From:   "Takiguchi, Yasunari" <Yasunari.Takiguchi@...y.com>
To:     Mauro Carvalho Chehab <mchehab@...pensource.com>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
        "tbird20d@...il.com" <tbird20d@...il.com>,
        "frowand.list@...il.com" <frowand.list@...il.com>,
        "Yamamoto, Masayuki" <Masayuki.Yamamoto@...y.com>,
        "Nozawa, Hideki (STWN)" <Hideki.Nozawa@...y.com>,
        "Yonezawa, Kota" <Kota.Yonezawa@...y.com>,
        "Matsumoto, Toshihiko" <Toshihiko.Matsumoto@...y.com>,
        "Watanabe, Satoshi (SSS)" <Satoshi.C.Watanabe@...y.com>,
        "Takiguchi, Yasunari" <Yasunari.Takiguchi@...y.com>
Subject: RE: [PATCH v4 05/12] [media] cxd2880: Add tuner part of the driver

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