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
| ||
|
Date: Wed, 20 Dec 2017 00:54:48 +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 02/12] [media] cxd2880-spi: Add support for CXD2880 SPI interface Hi, Mauro. > > + > > +#define pr_fmt(fmt) KBUILD_MODNAME ": %s: " fmt, __func__ > > It would be better to use dev_foo() debug macros instead of > pr_foo() ones. I got comment for this previous version patch as below -------------------------------------------------------------------------------------- The best would be to se dev_err() & friends for printing messages, as they print the device's name as filled at struct device. If you don't use, please add a define that will print the name at the logs, like: #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt either at the begining of the driver or at some header file. Btw, I'm noticing that you're also using dev_err() on other places of the code. Please standardize. OK, on a few places, you may still need to use pr_err(), if you need to print a message before initializing struct device, but I suspect that you can init -------------------------------------------------------------------------------------- You pointed out here before. Because dev_foo () and pr_foo () were mixed. We standardize with pr_foo() because the logs is outputted before getting the device structure. Is it better to use dev_foo() where we can use it? > > +static int cxd2880_stop_feed(struct dvb_demux_feed *feed) { > > + int i = 0; > > + int ret; > > + struct dvb_demux *demux = NULL; > > + struct cxd2880_dvb_spi *dvb_spi = NULL; > > + > > + if (!feed) { > > + pr_err("invalid arg\n"); > > + return -EINVAL; > > + } > > + > > + demux = feed->demux; > > + if (!demux) { > > + pr_err("feed->demux is NULL\n"); > > + return -EINVAL; > > + } > > + dvb_spi = demux->priv; > > + > > + if (!dvb_spi->feed_count) { > > + pr_err("no feed is started\n"); > > + return -EINVAL; > > + } > > + > > + if (feed->pid == 0x2000) { > > + /* > > + * Special PID case. > > + * Number of 0x2000 feed request was stored > > + * in dvb_spi->all_pid_feed_count. > > + */ > > + if (dvb_spi->all_pid_feed_count <= 0) { > > + pr_err("PID %d not found.\n", feed->pid); > > + return -EINVAL; > > + } > > + dvb_spi->all_pid_feed_count--; > > + } else { > > + struct cxd2880_pid_filter_config cfgtmp; > > + > > + cfgtmp = dvb_spi->filter_config; > > + > > + for (i = 0; i < CXD2880_MAX_FILTER_SIZE; i++) { > > + if (feed->pid == cfgtmp.pid_config[i].pid && > > + cfgtmp.pid_config[i].is_enable != 0) { > > + cfgtmp.pid_config[i].is_enable = 0; > > + cfgtmp.pid_config[i].pid = 0; > > + pr_debug("removed PID %d from #%d\n", > > + feed->pid, i); > > + break; > > + } > > + } > > + dvb_spi->filter_config = cfgtmp; > > + > > + if (i == CXD2880_MAX_FILTER_SIZE) { > > + pr_err("PID %d not found\n", feed->pid); > > + return -EINVAL; > > + } > > + } > > + > > + ret = cxd2880_update_pid_filter(dvb_spi, > > + &dvb_spi->filter_config, > > + dvb_spi->all_pid_feed_count > > 0); > > + dvb_spi->feed_count--; > > + > > + if (dvb_spi->feed_count == 0) { > > + int ret_stop = 0; > > + > > + ret_stop = > kthread_stop(dvb_spi->cxd2880_ts_read_thread); > > + if (ret_stop) { > > + pr_err("'kthread_stop failed. (%d)\n", > ret_stop); > > + ret = ret_stop; > > + } > > + kfree(dvb_spi->ts_buf); > > + dvb_spi->ts_buf = NULL; > > + } > > + > > + pr_debug("stop feed ok.(count %d)\n", dvb_spi->feed_count); > > + > > + return ret; > > +} > > I have the feeling that I've seen the code above before at the dvb core. > Any reason why don't use the already-existing code at dvb_demux.c & > friends? The CXD2880 HW PID filter is used to decrease the amount of TS data transferred via limited bit rate SPI interface. Thanks, Takiguchi
Powered by blists - more mailing lists