Re: [PATCH] media:dvb-frontends:Return if Max devices are added in dvb_pll_attach().
Hi, > diff --git a/drivers/media/dvb-frontends/dvb-pll.c > b/drivers/media/dvb-frontends/dvb-pll.c > index ba0c49107bd2..c850f1d69bce 100644 > --- a/drivers/media/dvb-frontends/dvb-pll.c > +++ b/drivers/media/dvb-frontends/dvb-pll.c > @@ -788,6 +788,9 @@ struct dvb_frontend *dvb_pll_attach(struct dvb_frontend > *fe, int pll_addr, > int ret; > const struct dvb_pll_desc *desc; > > + if (dvb_pll_devcount > DVB_PLL_MAX - 1) > + return NULL; > + > b1 = kmalloc(1, GFP_KERNEL); > if (!b1) > return NULL; > Wouldn't it put a limit on the number of attachment of devices? I'm afraid that an user may repeatedly plugs in and off a device using this driver (for some reason), and finally gets an error. Since dvb_pll_devcount and "id" module parameter are just used for debugging purpose to override/force PLL type, removing them totally would be better, IMHO. regards, Akihiro
Re: [PATCH] media:dvb-frontends:Return if Max devices are added in dvb_pll_attach().
> Will it be better, if dvb_pll_devcount is decremented in dvb_pll_release(), > instead of removing module params? But you cannot know deterministically which device corrensponds to what "id" (when you have multiple dvb_pll devices), since "id" is dependent on the history of register and unregister of dvb_pll devices. So I wonder about the benefit / practical usecase of "id" parameter. -- Akihiro
Re: [PATCH] dvb-usb-v2/gl861: fix wrong memcpy
> Does anyone have this hardware? This device must have been dead for about > a year, ever since commit b30cc07de8a9 was merged. I have one. (and I wrote the patch). Since I do not use it regularly and my app did not use the return value meaningfully, I have not noticed. regards, Akihiro
Re: [PATCH] dvb-usb-v2/gl861: fix wrong memcpy
> Could you please test the patch and check if the return results are > now consistent and that it won't break anything? I have tested the patch and it worked without problems. Testd-by: Akihiro Tsukada I could not noticed the bug because the device was registered without any error messages, and it seemed to work even with the bug. (Though actually I was wrong and missed that the device does not work after reboot or re-plugging). After applying this patch, I have confirmed that the device now works after reboot/re-plugging without any problems. note: The patched func: gl861_i2c_read_ex was used in device's early init, called from d->props->power_ctrl (from dvb_usbv2_init). But dvb_usbv2_init does not check the return value of it, and if the device had been initialized previously it can work even with the interrupted init process in power_ctrl(). -- Akihiro
Re: [PATCH] dvb-usb-v2/gl861: fix wrong memcpy
Hi, > I suspect all whole friio_reset() function is not needed as it has > worked even I/O has been broken. It worked because the old driver (that I rmmod'ed before installing the testing driver) properly init'ed the device. If I re-plug it (or reboot), it does not work. So it is needed. > Also tuner I2C adapter is implemented wrong (I think I mentioned that > earlier). As tuner sits behind demod I2C-adapter/gate that whole logic > should be on demod driver. But according to USB packet capture logs of the windows version, it makes eccentric use of USB messages ('bRequest' field), that (I believe) necessitates the current implementation, as I mentioned in the past thread. Regards, Akihiro
Re: [PATCH] gl861: re-implement i2c adapter logic
Hi, thanks for the example patch. > Here is debug log I tested multibyte i2c writes using zl10353 demod. All > returned bytes are not same, but it due to write only register bits I > think. > > dvb_usb_gl861 1-13:1.0: 1 | c0 02 00 1e 50 00 01 00 <<< 03 > dvb_usb_gl861 1-13:1.0: 1 | c0 02 00 1e 51 00 01 00 <<< 44 > dvb_usb_gl861 1-13:1.0: 1 | c0 02 00 1e 52 00 01 00 <<< 46 > dvb_usb_gl861 1-13:1.0: 1 | c0 02 00 1e 53 00 01 00 <<< 15 > dvb_usb_gl861 1-13:1.0: 1 | c0 02 00 1e 54 00 01 00 <<< 0f > dvb_usb_gl861 1-13:1.0: 5 | 40 03 00 1e 50 00 05 00 >>> 0c 77 aa bb cc > dvb_usb_gl861 1-13:1.0: 1 | c0 02 00 1e 50 00 01 00 <<< 0c > dvb_usb_gl861 1-13:1.0: 1 | c0 02 00 1e 51 00 01 00 <<< 77 > dvb_usb_gl861 1-13:1.0: 1 | c0 02 00 1e 52 00 01 00 <<< aa > dvb_usb_gl861 1-13:1.0: 1 | c0 02 00 1e 53 00 01 00 <<< 3b > dvb_usb_gl861 1-13:1.0: 1 | c0 02 00 1e 54 00 01 00 <<< 4c > > > Now if you look your tuner i2c implementation... > > buf[0] = msg->addr << 1; > memcpy(buf + 1, msg->buf, msg->len); > ret = usb_control_msg(d->udev, usb_sndctrlpipe(d->udev, 0), > GL861_REQ_I2C_RAW, GL861_WRITE, > priv->i2c_client_demod->addr << (8 + 1), 0xFE, buf, msg->len + 1, 2000); > > ...it translates same. Log of an 1-byte read from tuner in Friio looks like the following: (re-formatted from my past post: https://patchwork.linuxtv.org/comment/92946/ ) 40 03 00 30 fe 00 01 00 >>> c1 # command a read from the tuner@0x60 (hence 0xc1) c0 02 00 30 00 01 01 00 <<< 7c # get the result (return value: 0x7c) so, - One read is composed of *two* USB messages. (note that friio_tuner_i2c_xfer() does NOT combine the two I2C messages of one read, and issues separate USB message for each, contrary to gl861_i2c_master_xfer()). - The second USB message uses CMD_READ but 'index'(demod register addr) value exceeds 8bit (0x0100), thus cannot use the normal gl861_i2c_master_xfer() as is. It looks to me different. Regards, Akihiro
Re: [PATCH] gl861: re-implement i2c adapter logic
Hi, thanks for the patch. > See updated patch on ml. This raises two concerns for me: [1]. Clients must split an I2C transaction of one read into two, releasing the lock between them. They might be interrupted (for example) by other read transaction to another tuner, (or by other, un-related I2C to the demod), and get wrong result. ex. 1. write a read command to tuner@ 40 03 00 30 fe 00 01 00 >>> (XX << 1) | 1 2. write another read command to tuner@ 40 03 00 30 fe 00 01 00 >>> (YY << 1) | 1 3. read the result from tuner@XX C0 02 00 30 00 01 01 00 <<< ??? # which one? 4. read the result from tuner@YY C0 02 00 30 00 01 01 00 <<< ??? # right/valid answer? [2]. There are (fairly common) use-cases of 2-Bytes, non-short writing, which is not seemed to be supported by this patch. ex. tuner read log from my previous post. (no tuner register) 40 03 00 30 fe 00 01 00 >>> c1 # addr:0x18, buf = {0xfe, 0xc1} And friio itself also uses those 2-Bytes non-short msgs in its reset, to control some peripherals other than demod/tuners. ex. gl861.c::friio_reset()::line 422 40 03 00 12 03 00 01 00 >>> 80 # addr:0x09, buf = {0x03, 0x80} But It may be possible to get around all these concerns. For [1], demod usually does not share tuner I2C access like this. For example, tc90522 supports multiple tuner input but has separate demod blocks for each input with distinct I2C address. For [2], 'short write' version might work still. (not sure/verified) (40 02 c1 30 fe 00 00 00 >>>) And friio can use gl861_ctrl_msg() directly instead. So, anyway I will try to re-structure i2c in friio and test/verify it. Regards, Akihiro
Re: [media 2/6] drop backstabbing drivers
You still haven't described any concrete reasons why you revert those existing drivers, and I definitely disagree with this patch. -- akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] em28xx_dvb: add support for PLEX PX-BCUD (ISDB-S usb dongle)
Hi, Satoshi, just some small comments about tuners/qm1d1c0042... > --- a/drivers/media/tuners/qm1d1c0042.c > +++ b/drivers/media/tuners/qm1d1c0042.c > @@ -32,14 +32,23 @@ > #include "qm1d1c0042.h" > > #define QM1D1C0042_NUM_REGS 0x20 > - > -static const u8 reg_initval[QM1D1C0042_NUM_REGS] = { > -0x48, 0x1c, 0xa0, 0x10, 0xbc, 0xc5, 0x20, 0x33, > -0x06, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, > -0x00, 0xff, 0xf3, 0x00, 0x2a, 0x64, 0xa6, 0x86, > -0x8c, 0xcf, 0xb8, 0xf1, 0xa8, 0xf2, 0x89, 0x00 > +#define QM1D1C0042_NUM_REG_ROWS 2 > + > +static const u8 > reg_initval[QM1D1C0042_NUM_REG_ROWS][QM1D1C0042_NUM_REGS] = { { > +0x48, 0x1c, 0xa0, 0x10, 0xbc, 0xc5, 0x20, 0x33, > +0x06, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, > +0x00, 0xff, 0xf3, 0x00, 0x2a, 0x64, 0xa6, 0x86, > +0x8c, 0xcf, 0xb8, 0xf1, 0xa8, 0xf2, 0x89, 0x00 > +}, { > +0x68, 0x1c, 0xc0, 0x10, 0xbc, 0xc1, 0x11, 0x33, > +0x03, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, > +0x00, 0xff, 0xf3, 0x00, 0x3f, 0x25, 0x5c, 0xd6, > +0x55, 0xcf, 0x95, 0xf6, 0x36, 0xf2, 0x09, 0x00 > +} > }; > > +static int reg_index; > + * The names of _REG_ROWS / reg_index might be a bit vague to others. I would prefer _CHIP_IDS / chip_id or something like that. * reg_index should not be static as it is per device property. Instead, it shouldj be defined in qm1d1c0042_init() locally, or in struct qm1d1c0042_state, if "reg_index" can be used elsewhere. Thre rest looks OK to me. regards, akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
dvb-core: how should i2c subdev drivers be attached?
Hi, excuse me for taking up a very old post again, but I'd like to know the status of the patch: https://patchwork.linuxtv.org/patch/27922/ , which provides helper code for defining/loading i2c DVB subdev drivers. Was it rejected and each i2c demod/tuner drivers should provide its own version of "attach" code? Or is it acceptable (with some modifications) ? Although not many drivers currently use i2c binding model (and use dvb_attach()), but I expect that coming DVB subdev drivers will have a similar attach code, including module request/ref-counting, device creation, (re-)using i2c_board_info.platformdata to pass around both config parameters and the resulting i2c_client* & dvb_frontend*. Since I have a plan to split out demod/tuner drivers from pci/pt1 dvb-usb/friio integrated drivers (because those share the tc90522 demod driver with pt3, and friio also shares the bridge chip with gl861), it would be nice if I can use the helper code, instead of re-iterating similar "attach" code. -- akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: dvb-core: how should i2c subdev drivers be attached?
thanks for the replys. so, the new i2c "attach" helper code should 1. be usable for V4L2-DVB multi-functional(hybrid) tuner devices (or at least co-exist well with them), 2. properly ref-count driver module to prevent (accidental) 'unload before use' by users. 3. a. block un-binding of the device while in use, b. support run-time {un-,}binding of the devices via sysfs by users (to switch drivers?). Is this right? > I guess we should discuss a way of doing it that will be acceptable > on existing drivers. Perhaps you should try to do such change for > an hybrid driver like em28xx or cx231xx. There are a few ISDB-T > devices using them. Not sure how easy would be to find one of those > in Japan, though. I'll look into those drivers' code, to begin with. (I'm pretty sure those devices are hardly found here in Japan). > I'm now helping to to maintain Kaffeine upstream. I recently added > support for both ISDB-T and DVB-T2. It would be nice if you could > add support there for ISDB-S too. All the channels in ISDB-S are scrambled, unlike ISDB-T(jp) where non-scrambled '1-seg' channels are delivered. In Japan, it will be considered illegal to desramble them with a non-authorized software that lacks private copy-guard encryption. So, unfortunately, there will be no legitimate OSS player for ISDB-S. regards, akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dvb_frontend: Multistream support
Hi, > diff --git a/drivers/media/dvb/dvb-core/dvb_frontend.h > b/drivers/media/dvb/dvb-core/dvb_frontend.h > index 7c64c09..bec0cda 100644 > --- a/drivers/media/dvb/dvb-core/dvb_frontend.h > +++ b/drivers/media/dvb/dvb-core/dvb_frontend.h > @@ -368,11 +368,8 @@ struct dtv_frontend_properties { > u8 interleaving; > } layer[3]; > > - /* ISDB-T specifics */ > - u32 isdbs_ts_id; > - > - /* DVB-T2 specifics */ > - u32 dvbt2_plp_id; > + /* Multistream specifics */ > + u32 stream_id; > > /* ATSC-MH specifics */ > u8 atscmh_fic_ver; It would be nice if you would include a patch to replace isdbs_ts_id in dvb/pt1/va1j5jf8007s.c, which is the only file to use the variable. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Full DVB driver package for Earthsoft PT3 (ISDB-S/ISDB-T) cards.
Hi, It would be nice if you consider integrating this driver and pt1. Because PT3 is a PCIe successor of PT2 and designed by the same person, I guess they can share much code about bridge FPGA chip and demod IC (tc90512 in PT1/2 and tc90522 in PT3), though their frontends are obviously different. regards, Akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ISDB-T tuner
> My tuner card is a Pixela PIXDT090-PE0 Hi Romont, As you might know, all Japanese DTV programs are scrambled with BCAS. BCAS scrambling algorithm itself is opend in the standard, but in addition to that, PC devices have to encrypt received data locally in order to get authorized for BCAS. So most DTV devices sold in Japan cannot be used in Linux. Some self-started vendors have sold their devices without BCAS authentication (thus without local encryption). They don't/can't include a BCAS PC-card necessary for descrambling, and users must bring it from elsewhere, which is against the EULA with the exclusive and private card issuer organization, or just live with qcif-sized non scrambled 'one seg.' programs in TS. This is why so few ISDB-T/S devices are supported in Linux. And Pixela is one of the major vendors with BCAS authentication. So I'm afraid there is almost no possibility to be supported in Linux. And just for you information, in addition to EarthSoft PT1, there is a driver for 'Friio' ISDB-T USB receiver (which I wrote;) , and it is already included in the main repository. Dibcom is maybe for Brazil and may or may not work in Japan. (Some of the SKnet HDUS series USB receivers are known to be hack-able to avoid local encryption, but of course it's underground.) -- tsukada -- GyaO! - Anime, Dramas, Movies, and Music videos [FREE] http://pr.mail.yahoo.co.jp/gyao/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re : ISDB-T tuner
> so, which device can I buy, working in Japan, in linux, and in digital > (working after 2011)? known to work devices 0. 774 Friio white http://www.friio.com/ (in Japanese) USB2.0 ISDB-T, with a card-reader device, without BCAS card DVB driver: in v4l-dvb main repository direct net shopping only, pretty expensive. card reader is very unstable due to its large initial power consumption, and pratically needs a self-powered USB hub to used this reader. (receiver itself is OK in bus-powered) 1. Earthsoft PT1 http://earthsoft.jp/PT/index.html (in Japanese) PCI 2xISDB-T + 2xISDB-S, 4 concurrent streaming no BCAS card, no card-reader. DVB driver: in v4l-dvb main repository very popular in this area but discontinued. PT2 has been released but highly out of stock, and no driver yet. 2. SKNet MonsterTV HDUS, or HDUSF http://www.sknet-web.co.jp/product/mtvhdus.html USB2.0 ISDB-T, with BCAS card&reader, remote-controller a bit old but maybe most available in stores. I don't know much about this device, but it is reported to work in Linux, (with some firmware hack?) and some people seems to have written DVB driver. you may find it from http://2sen.dip.jp/cgi-bin/hdusup/upload.cgi (up0432.zip??) I heard that some another version(series?) have internal encryption and are difficult to make it work in Linux. note 0. DVB drivers output scrambled streams as is. an user-land application is necessary to descramble. stand-alone descrambler: http://www.marumo.ne.jp/junk/arib_std_b25-0.2.4.lzh patch to mplayer/gstreamer demuxer: http://2sen.dip.jp/cgi-bin/dtvup/source/up0176.zip note 1. you have to prepare yourself PC/SC card reader and a BCAS card if the device does not bundle them. I guess you already have a BCAS card in hand already;) Card reader must be supported by PC/SC to be used with the above descrambling applications. for example, http://www.amazon.co.jp/gp/switch-language/product/B001NEIRH0/ref=dp_change_lang?ie=UTF8&language=en_JP is told to work, but I'm not sure. note 2. re-using a BCAS card for non-autorized (bundeled) devices is probably against the EULA of the card. (one seg. programs are not scrambled and can be viewed without a BCAS card). - akihiro -- GyaO! - Anime, Dramas, Movies, and Music videos [FREE] http://pr.mail.yahoo.co.jp/gyao/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] dvb-usb-friio: accept center-shifted frequency
From: Akihiro Tsukada This patch adds a fix to accept frequency with its center shifted. The driver used to accept center frequencies of the normal UHF band channels, but in ISDB-T, center frequency is shifted with 1/7MHz. It was shifted internally in the driver, but this patch enables to accept both types of frequency. Priority: normal Signed-off-by: Akihiro Tsukada --- friio-fe.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/linux/drivers/media/dvb/dvb-usb/friio-fe.c b/linux/drivers/media/dvb/dvb-usb/friio-fe.c --- a/linux/drivers/media/dvb/dvb-usb/friio-fe.c +++ b/linux/drivers/media/dvb/dvb-usb/friio-fe.c @@ -134,11 +134,13 @@ static int jdvbt90502_pll_set_freq(struc deb_fe("%s: freq=%d, step=%d\n", __func__, freq, state->frontend.ops.info.frequency_stepsize); /* freq -> oscilator frequency conversion. */ - /* freq: 473,000,000 + n*6,000,000 (no 1/7MHz shift to center freq) */ - /* add 400[1/7 MHZ] = 57.142857MHz. 57MHz for the IF, */ - /* 1/7MHz for center freq shift */ + /* freq: 473,000,000 + n*6,000,000 [+ 142857 (center freq. shift)] */ f = freq / state->frontend.ops.info.frequency_stepsize; - f += 400; + /* add 399[1/7 MHZ] = 57MHz for the IF */ + f += 399; + /* add center frequency shift if necessary */ + if (f % 7 == 0) + f++; pll_freq_cmd[DEMOD_REDIRECT_REG] = JDVBT90502_2ND_I2C_REG; /* 0xFE */ pll_freq_cmd[ADDRESS_BYTE] = state->config.pll_address << 1; pll_freq_cmd[DIVIDER_BYTE1] = (f >> 8) & 0x7F; -- GyaO! - Anime, Dramas, Movies, and Music videos [FREE] http://pr.mail.yahoo.co.jp/gyao/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] dvb-apps ported for ISDB-T
Hi Mauro, >> I wrote several patches those days in order to allow dvb-apps to properly >> parse ISDB-T channel.conf. I think it would be convenient if channel.conf allows the kind of format of PROPNAME=VALUE list, for readability and extensibility of the conf file. there are already so many parameters in ISDB-T, so it is a bit difficult for the users to remember and correctly specify all the field in the right order. besides they have to be careful in counting the delimiter character when some (most?) parameters can be omitted or be left to the device/region default. and when I consider extending this lib to ISDB-S (for example), I have to add "TS-id" parameter, which leads to the re-definition of the data structure and requires re-building. So, it would be convenient if I could write for example like, DTV_FREQUENCY=1049480:DTV_ISDBS_TS_ID=1 regards, akihiro -- Get Disney character's mail address on Yahoo! Mail http://pr.mail.yahoo.co.jp/disney/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: dvbzap application based on DVBv5 API
> [channel name] > property = value > ... > property = value Currently, at least gstreamer's dvbbasebin and mplayer assumue that the channel configuration file has the format of one line per channel. So when I personally patched them to use v5 parameters, I chose the one-line-per-channel format of :propname=val|...|propname=val:, for example, NHKBS1:DTV_DELIVERY_SYSTEM=SYS_ISDBS|DTV_VOLTAGE=1|DTV_FREQUENCY=1318000|DTV_ISDBS_TS_ID=0x40f2:103 , to minimize modification (hopefully). I understand that it is not that difficult nor complicated to adapt applications to use the ini file style format, but the old one line style format seems slightly easier. and I wish that the channel configuration can allow nicknames/aliases, as the canonical channel name can be long to type in or difficult to remember correctly. If I remember right, MythTV has its own database, and it would be convenient if we could share the database, because applications currently have their own channel configuration separately, and the configuration change like new service or parameter changes must be propagated manually. regards, Akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: adding ISDB-T/ISDB-Tsb to DVB-API 5
Hi Patrick, Thank you for your effort to add support for ISDB-T/S. I've skimmed through the ARIB standard before, but it is too complicated for me to understand well enough. So this is not a comment for the API extension itself, but for the document part. Some of the parameters are currently (and probably will stay) fixed or not used according to the "operational guidelines". For example, DQPSK is not used at all (if I read correctly). These guidelines are defined in ARIB TR-B14 for ISDB-T and in ARIB TR-B15 for ISDB-S respectively. So, including these two TRs (in additino to ARIB STD-B31) as a reference in the document may help readers. ----- Akihiro TSUKADA -- Power up the Internet with Yahoo! Toolbar. http://pr.mail.yahoo.co.jp/toolbar/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] dvb: add driver for 774 Friio White USB ISDB-T receiver
From: Akihiro Tsukada This patch adds driver for 774 Friio White, ISDB-T USB receiver Friio White is an USB 2.0 ISDB-T receiver. (http://www.friio.com/) The device has a GL861 chip and a Comtech JDVBT90502 canned tuner module. This driver ignores all the frontend_parameters except frequency, as ISDB-T shares the same parameter configuration across the country and thus the device can work like an intelligent one. As this device does not include a CAM nor hardware descrambling feature, the driver passes through scrambled TS streams. There is Friio Black, a variant for ISDB-S, which shares the same USB Vendor/Product ID with White, but it is not supported in this driver. They should be identified in the initialization sequence, but this feature is not tested. Priority: normal Signed-off-by: Akihiro Tsukada --- Currently, all the Japanese Digital TV broadcasters, including the charge-free ones, scramble their programs except the tiny sized versions for cellular phones (called "1 seg." program). "BCAS" card is used to descramble those normal sized programs, and a private agency named "BS Conditional Systems" exclusively issues the cards only as a bundle for the approved receiver devices. Friio White is not, so it does not include a BCAS card. There are some descrambler software using a BCAS card from somewhere else with a PC/SC reader. Examples of these software include, filter command: http://www.marumo.ne.jp/junk/arib_std_b25-0.2.4.lzh Gstreamer plugin: http://2sen.dip.jp/cgi-bin/dtvup/source/up0158.zip But you have to note that their use may violate the card's license agreement. diff --git a/linux/drivers/media/dvb/dvb-usb/Kconfig b/linux/drivers/media/dvb/dvb-usb/Kconfig --- a/linux/drivers/media/dvb/dvb-usb/Kconfig +++ b/linux/drivers/media/dvb/dvb-usb/Kconfig @@ -315,3 +315,9 @@ config DVB_USB_CE6230 select MEDIA_TUNER_MXL5005S if !MEDIA_TUNER_CUSTOMISE help Say Y here to support the Intel CE6230 DVB-T USB2.0 receiver + +config DVB_USB_FRIIO + tristate "Friio ISDB-T USB2.0 Receiver support" + depends on DVB_USB + help + Say Y here to support the Japanese DTV receiver Friio. diff --git a/linux/drivers/media/dvb/dvb-usb/Makefile b/linux/drivers/media/dvb/dvb-usb/Makefile --- a/linux/drivers/media/dvb/dvb-usb/Makefile +++ b/linux/drivers/media/dvb/dvb-usb/Makefile @@ -79,6 +79,9 @@ obj-$(CONFIG_DVB_USB_CINERGY_T2) += dvb- dvb-usb-ce6230-objs = ce6230.o obj-$(CONFIG_DVB_USB_CE6230) += dvb-usb-ce6230.o +dvb-usb-friio-objs = friio.o friio-fe.o +obj-$(CONFIG_DVB_USB_FRIIO) += dvb-usb-friio.o + EXTRA_CFLAGS += -Idrivers/media/dvb/dvb-core/ -Idrivers/media/dvb/frontends/ # due to tuner-xc3028 EXTRA_CFLAGS += -Idrivers/media/common/tuners diff --git a/linux/drivers/media/dvb/dvb-usb/dvb-usb-ids.h b/linux/drivers/media/dvb/dvb-usb/dvb-usb-ids.h --- a/linux/drivers/media/dvb/dvb-usb/dvb-usb-ids.h +++ b/linux/drivers/media/dvb/dvb-usb/dvb-usb-ids.h @@ -59,6 +59,7 @@ #define USB_VID_YUAN 0x1164 #define USB_VID_XTENSIONS 0x1ae7 #define USB_VID_HUMAX_COEX 0x10b9 +#define USB_VID_7740x7a69 /* Product IDs */ #define USB_PID_ADSTECH_USB2_COLD 0xa333 @@ -264,5 +265,6 @@ #define USB_PID_ELGATO_EYETV_DTT_Dlx 0x0020 #define USB_PID_DVB_T_USB_STICK_HIGH_SPEED_COLD0x5000 #define USB_PID_DVB_T_USB_STICK_HIGH_SPEED_WARM0x5001 +#define USB_PID_FRIIO_WHITE0x0001 #endif diff --git a/linux/drivers/media/dvb/dvb-usb/friio-fe.c b/linux/drivers/media/dvb/dvb-usb/friio-fe.c new file mode 100644 --- /dev/null +++ b/linux/drivers/media/dvb/dvb-usb/friio-fe.c @@ -0,0 +1,483 @@ +/* DVB USB compliant Linux driver for the Friio USB2.0 ISDB-T receiver. + * + * Copyright (C) 2009 Akihiro Tsukada + * + * This module is based off the the gl861 and vp702x modules. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation, version 2. + * + * see Documentation/dvb/README.dvb-usb for more information + */ +#include +#include +#include + +#include "friio.h" + +struct jdvbt90502_state { + struct i2c_adapter *i2c; + struct dvb_frontend frontend; + struct jdvbt90502_config config; +}; + +/* NOTE: TC90502 has 16bit register-address? */ +/* register 0x0100 is used for reading PLL status, so reg is u16 here */ +static int jdvbt90502_reg_read(struct jdvbt90502_state *state, + const u16 reg, u8 *buf, const size_t count) +{ + int ret; + u8 wbuf[3]; + struct i2c_msg msg[2]; + + wbuf[0] = reg & 0xFF; + wbuf[1] = 0; + wbuf[2] = reg >> 8; + + msg[0].addr = state->config.demod_address; + msg[0].fl
[PATCH 1/2] dvb-usb-friio: return the correct DTV_DELIVERY_SYSTEM
From: Akihiro Tsukada This patch makes the driver return the correct DTV_DELIVERY_SYSTEM. The driver previously returned SYS_UNDEFINED for DTV_DELIVERY_SYSTEM property, as it lacked any driver specific S2API support. Priority: normal Signed-off-by: Akihiro Tsukada --- friio-fe.c | 27 +++ 1 file changed, 27 insertions(+) diff --git a/linux/drivers/media/dvb/dvb-usb/friio-fe.c b/linux/drivers/media/dvb/dvb-usb/friio-fe.c --- a/linux/drivers/media/dvb/dvb-usb/friio-fe.c +++ b/linux/drivers/media/dvb/dvb-usb/friio-fe.c @@ -286,6 +286,27 @@ static int jdvbt90502_get_tune_settings( return 0; } +/* filter out un-supported properties to notify users */ +static int jdvbt90502_set_property(struct dvb_frontend *fe, + struct dtv_property *tvp) +{ + int r = 0; + + switch (tvp->cmd) { + case DTV_DELIVERY_SYSTEM: + if (tvp->u.data != SYS_ISDBT) + r = -EINVAL; + break; + case DTV_CLEAR: + case DTV_TUNE: + case DTV_FREQUENCY: + break; + default: + r = -EINVAL; + } + return r; +} + static int jdvbt90502_get_frontend(struct dvb_frontend *fe, struct dvb_frontend_parameters *p) { @@ -314,6 +335,9 @@ static int jdvbt90502_set_frontend(struc deb_fe("%s: Freq:%d\n", __func__, p->frequency); + /* for recovery from DTV_CLEAN */ + fe->dtv_property_cache.delivery_system = SYS_ISDBT; + ret = jdvbt90502_pll_set_freq(state, p->frequency); if (ret) { deb_fe("%s:ret == %d\n", __func__, ret); @@ -394,6 +418,7 @@ static int jdvbt90502_init(struct dvb_fr if (ret != 1) goto error; } + fe->dtv_property_cache.delivery_system = SYS_ISDBT; msleep(100); return 0; @@ -471,6 +496,8 @@ static struct dvb_frontend_ops jdvbt9050 .sleep = jdvbt90502_sleep, .write = _jdvbt90502_write, + .set_property = jdvbt90502_set_property, + .set_frontend = jdvbt90502_set_frontend, .get_frontend = jdvbt90502_get_frontend, .get_tune_settings = jdvbt90502_get_tune_settings, -- Yahoo! JAPAN - Internet Security for teenagers and parents. http://pr.mail.yahoo.co.jp/security/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] dvb-usb-friio: cleaning up unnecessary functions
From: Akihiro Tsukada This patch removes some fe->ops.X() functions which do nothing more useful than the default. Priority: normal Signed-off-by: Akihiro Tsukada --- friio-fe.c | 38 -- 1 file changed, 38 deletions(-) diff --git a/linux/drivers/media/dvb/dvb-usb/friio-fe.c b/linux/drivers/media/dvb/dvb-usb/friio-fe.c --- a/linux/drivers/media/dvb/dvb-usb/friio-fe.c +++ b/linux/drivers/media/dvb/dvb-usb/friio-fe.c @@ -232,12 +232,6 @@ static int jdvbt90502_read_status(struct return 0; } -static int jdvbt90502_read_ber(struct dvb_frontend *fe, u32 *ber) -{ - *ber = 0; - return 0; -} - static int jdvbt90502_read_signal_strength(struct dvb_frontend *fe, u16 *strength) { @@ -264,27 +258,6 @@ static int jdvbt90502_read_signal_streng return 0; } -static int jdvbt90502_read_snr(struct dvb_frontend *fe, u16 *snr) -{ - *snr = 0x0101; - return 0; -} - -static int jdvbt90502_read_ucblocks(struct dvb_frontend *fe, u32 *ucblocks) -{ - *ucblocks = 0; - return 0; -} - -static int jdvbt90502_get_tune_settings(struct dvb_frontend *fe, - struct dvb_frontend_tune_settings *fs) -{ - fs->min_delay_ms = 500; - fs->step_size = 0; - fs->max_drift = 0; - - return 0; -} /* filter out un-supported properties to notify users */ static int jdvbt90502_set_property(struct dvb_frontend *fe, @@ -347,12 +320,6 @@ static int jdvbt90502_set_frontend(struc return 0; } -static int jdvbt90502_sleep(struct dvb_frontend *fe) -{ - deb_fe("%s called.\n", __func__); - return 0; -} - /** * (reg, val) commad list to initialize this module. @@ -493,18 +460,13 @@ static struct dvb_frontend_ops jdvbt9050 .release = jdvbt90502_release, .init = jdvbt90502_init, - .sleep = jdvbt90502_sleep, .write = _jdvbt90502_write, .set_property = jdvbt90502_set_property, .set_frontend = jdvbt90502_set_frontend, .get_frontend = jdvbt90502_get_frontend, - .get_tune_settings = jdvbt90502_get_tune_settings, .read_status = jdvbt90502_read_status, - .read_ber = jdvbt90502_read_ber, .read_signal_strength = jdvbt90502_read_signal_strength, - .read_snr = jdvbt90502_read_snr, - .read_ucblocks = jdvbt90502_read_ucblocks, }; -- Yahoo! JAPAN - Internet Security for teenagers and parents. http://pr.mail.yahoo.co.jp/security/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 3/5] dvb-usb/friio, dvb-usb-v2/gl861: decompose friio and merge with gl861
Hi, thanks for the review. >> +gl861_i2c_rawwrite(struct dvb_usb_device *d, u8 addr, u8 *wbuf, u16 wlen)>> >> +{>> + u8 *buf;>> +int ret;>> +>> +buf = kmalloc(wlen, >> GFP_KERNEL);>> +if (!buf)>> + return -ENOMEM;>> +>> + >> usleep_range(1000, 2000); /* avoid I2C errors */> > I guess this is probably >> also at the original code, but it seems really> weird to sleep here just >> after kmalloc().> > I would move any need for sleeps to the i2c xfer >> function, where it> makes clearer why it is needed and where. Same applies >> to other> usleep_range() calls at the functions below. those sleeps are for placing time gap between consecutive i2c transactions, but I agree that they should be moved to the i2c_xfer loop. I'll fix them in the next version. >> +/* >> + * In Friio, >> + * I2C commnucations to the tuner are relay'ed via the demod (via the >> bridge), >> + * so its encapsulation to USB message is different from the one to the >> demod. > > This is quite common. I guess the rationale of using the demod's I2C mux > is to avoid noise at the tuner when there are I2C traffic that aren't related > to it. > > You should probably implement it via I2C_MUX support. I know that the i2c bus structure is common, but as I wrote to Antti before, i2c transactions to the tuner use the different USB transactions with different 'request' value from the one used in the other demod transactions. Here is the packet log example (I posted to linux-media before): "Re: [PATCH v4] dvb-usb/friio, dvb-usb-v2/gl861: decompose friio and merge with gl861" Message ID: Sent on: Sun, 1 Apr 2018 00:30:49 +0900 I still do not understand how this can be implemented with I2C_MUX, or in the demod/tuner drivers. i2c to the tuner does not need gating/arbitration/locking, only needs to access via the demod reg, which is already implemented in the adapter by the demod, but it must be sent in the distinct USB transaction with its own 'request' value. >> +/* init/config of Friio demod chip(?) */ >> +static int friio_reset(struct dvb_usb_device *d) >> +{ >> +int i, ret; >> +u8 wbuf[2], rbuf[2]; >> + >> +static const u8 friio_init_cmds[][2] = { >> +{0x33, 0x08}, {0x37, 0x40}, {0x3a, 0x1f}, {0x3b, 0xff}, >> +{0x3c, 0x1f}, {0x3d, 0xff}, {0x38, 0x00}, {0x35, 0x00}, >> +{0x39, 0x00}, {0x36, 0x00}, >> +}; >> + >> +ret = usb_set_interface(d->udev, 0, 0); >> +if (ret < 0) >> +return ret; >> + >> +wbuf[0] = 0x11; >> +wbuf[1] = 0x02; >> +ret = gl861_i2c_msg(d, 0x00, wbuf, 2, NULL, 0); >> +if (ret < 0) >> +return ret; >> +usleep_range(2000, 3000); >> + >> +wbuf[0] = 0x11; >> +wbuf[1] = 0x00; >> +ret = gl861_i2c_msg(d, 0x00, wbuf, 2, NULL, 0); >> +if (ret < 0) >> +return ret; >> +usleep_range(1000, 2000); > > Hmm... you had already usleeps() all over I2C xfer routines. Why adding > extra sleeps here? Those sleeps were added to (roughly) simulate the timing seen in the packet capture logs on a Windows box. They certainly include the time for application processing, but may include the necessary wait time after each command/i2c-transaction, so they are kept for safety. > Also, why isn't it calling i2c_transfer() instead of gl861_i2c_msg()? > Same applies to other similar calls. Because friio_reset can be called in the following trace: friio_reset friio_power_ctrl d->props->power_ctrl dvb_usbv2_device_power_ctrl dvb_usbv2_init and in dvb_usbv2_init, dvb_usbv2_device_power_ctrl is called BEFORE i2c adapter initialization (dvb_usbv2_i2c_init). >> +static int friio_tuner_detach(struct dvb_usb_adapter *adap) >> +{ >> +struct friio_priv *priv; >> + >> +priv = adap_to_priv(adap); >> +#ifndef CONFIG_MEDIA_ATTACH >> +/* Note: >> + * When CONFIG_MEDIA_ATTACH is defined, >> + * the tuner module is already "put" by the following call trace: >> + * >> + * symbol_put_addr(fe->ops.tuner_ops.release) >> + * dvb_frontend_invoke_release(fe, fe->ops.tuner_ops.release) >> + * dvb_frontend_detach(fe) >> + * dvb_usbv2_adapter_frontend_exit(adap) >> + * dvb_usbv2_adapter_exit(d) >> + * dvb_usbv2_exit(d) >> + * dvb_usbv2_disconnect(intf) >> + * >> + * Since the tuner driver of Friio (dvb-pll) has .release defined, >> + * dvb_module_release() should be skipped if CONFIG_MEDIA_ATTACH, >> + * to avoid double-putting the module. >> + * Even without dvb_module_release(), >> + * the tuner i2c device is automatically unregistered >> + * when its i2c adapter is unregistered with the demod i2c device. >> + * >> + * The same "double-putting" can happen to the demod module as well, >> + * but tc90522 does not define any _invoke_release()'ed functions, >> + * thus Friio is not affected. >> + */ >> +dvb_module_release(priv->i2c_client_tuner); >> +#endif > > This looks really weird to me... why only at this driv
Re: I2C media binding model
Hi, As the new i2c binding helper was introduced, I am now re-writing the following patches to use new helper functions. > 5. Jan,16 2015: [v2,1/2] dvb: tua6034: add a new driver for Infineon tua6034 > tuner > http://patchwork.linuxtv.org/patch/27927 > 6. Jan,16 2015: [v2,2/2] dvb-usb-friio: split and merge into dvb-usbv2-gl861 > http://patchwork.linuxtv.org/patch/27928 But I noticed that the tua6034 (used in Friio devices) can be supported by "tuner-simple" driver. Since "tuner-simple" is not an i2c driver, I am wondering if 1) I should use/modify "tuner-simple" driver without adding new one but with one more dvb_attach un-replaced, or, 2) I should make a new i2c driver as my previous patch #27927 but with the duplicated function with tuner-simple. Re-writing tuner-simple into a new i2c driver does not seem to be an option to me, because that would affect lots of dvb/v4l drivers, and I cannot test them. In addition, I also intend to re-write "earth-pt1" driver in the future by decomposing the current monolithic module into component drivers: bridge: earth-pt1 (cut down one) demod: tc90522 TERR-tuner: tda6654 (NEW?) SAT-tuner: qm1d1b0004 (NEW) There exists a "tda665x" tuner driver (in dvb-frontends/), but it does not use the new i2c binding helpers either, and it seems that it can be supported by "tuner-simple" as well. So the similar situation here, though the tda665x driver is used only by "mantis" currently. So, which way should I go? modify/use "tuner-simple" for now and update to a i2c driver later? or introduce a new & redundant i2c driver? regards, Akihiro
Re: [PATCH v4] dvb-usb/friio, dvb-usb-v2/gl861: decompose friio and merge with gl861
Hi, thanks for the comment. > You should implement i2c adapter to demod driver and not add such glue > to that USB-bridge. I mean that "relayed" stuff, i2c communication to > tuner via demod. I2C-mux may not work I think as there is no gate-style > multiplexing so you probably need plain i2c adapter. There is few > examples already on some demod drivers. I am afraid that the glue is actually necessary. host - USB -> gl861 - I2C(1) -> tc90522 (addr:X) \- I2C(2) -> tua6034 (addr:Y) To send an i2c read message to tua6034, one has to issue two transactions: 1. write via I2C(1) to addr:X, [ reg:0xfe, val: Y ] 2. read via I2C(1) from addr:X, [ out_data0, out_data1, ] The problem is that the transaction 1 is (somehow) implemented with the different USB request than the other i2c transactions on I2C(1). (this is confirmed by a packet capture on Windows box). Although tc90522 already creats the i2c adapter for I2C(2), tc90522 cannot know/control the USB implementation of I2C(1), only the bridge driver can do this. regards, Akihiro
Re: [PATCH v4] dvb-usb/friio, dvb-usb-v2/gl861: decompose friio and merge with gl861
> I simply cannot see why it cannot work. Just add i2c adapter and > suitable logic there. Transaction on your example is simply and there is > no problem to implement that kind of logic to demod i2c adapter. I might be totally wrong, but... i2c transactions to a tuner must use: 1. usb_control_msg(request:3) for the first half (write) of reads 2. usb_control_msg(request:1) for the other writes 3. usb_control_msg(request:2) for (all) reads How can the demod driver control the 'request' argument of USB messages that are sent to its parent (not to the demod itself), when the bridge of tc90522 cannot be limited to gl861 (or even to USB) ? Akihiro
Re: [PATCH v4] dvb-usb/friio, dvb-usb-v2/gl861: decompose friio and merge with gl861
> I don't understand those control message parts and it is bit too hard to > read i2c adapter implementation to get understanding. Could you offer > simple 2 sniff examples, register write to demod and register write to > tuner. Here is the part of a packet log. 1. write to demod (addr:0x18) reg:0x76 val:0c === [46264 ms] >>> URB 146 going down >>> -- URB_FUNCTION_VENDOR_DEVICE: TransferFlags = (USBD_TRANSFER_DIRECTION_OUT, ~USBD_SHORT_TR ANSFER_OK) TransferBufferLength = TransferBuffer = 8609d21e TransferBufferMDL= UrbLink = RequestTypeReservedBits = Request = 0001 Value = 300c Index = 0076 [46266 ms] UsbSnoop - MyInternalIOCTLCompletion(f79b7db0) : fido=, Irp=8 58f2938, Context=858c4ed8, IRQL=2 [46266 ms] <<< URB 146 coming back <<< -- URB_FUNCTION_CONTROL_TRANSFER: PipeHandle = 86239260 TransferFlags= 000a (USBD_TRANSFER_DIRECTION_OUT, USBD_SHORT_TRANS FER_OK) TransferBufferLength = TransferBuffer = 8609d21e TransferBufferMDL= UrbLink = SetupPacket = : 40 01 0c 30 76 00 00 00 === 2. write to tuner (addr:0x60) write [0f 7b b2 08] to addr 0x60 === [47267 ms] >>> URB 147 going down >>> -- URB_FUNCTION_VENDOR_DEVICE: TransferFlags = (USBD_TRANSFER_DIRECTION_OUT, ~USBD_SHORT_TR ANSFER_OK) TransferBufferLength = 0005 TransferBuffer = 8581c7d6 TransferBufferMDL= : c0 0f 7b b2 08 UrbLink = RequestTypeReservedBits = Request = 0003 Value = 3000 Index = 00fe [47270 ms] UsbSnoop - MyInternalIOCTLCompletion(f79b7db0) : fido=, Irp=8 58f2008, Context=86275258, IRQL=2 [47270 ms] <<< URB 147 coming back <<< -- URB_FUNCTION_CONTROL_TRANSFER: PipeHandle = 86239260 TransferFlags= 000a (USBD_TRANSFER_DIRECTION_OUT, USBD_SHORT_TRANS FER_OK) TransferBufferLength = 0005 TransferBuffer = 8581c7d6 TransferBufferMDL= 855f7760 UrbLink = SetupPacket = : 40 03 00 30 fe 00 05 00 === 3. read from tuner read one byte from addr 0x60 (2 USB packets) === [46036 ms] >>> URB 26 going down >>> -- URB_FUNCTION_VENDOR_DEVICE: TransferFlags = (USBD_TRANSFER_DIRECTION_OUT, ~USBD_SHORT_TR ANSFER_OK) TransferBufferLength = 0001 TransferBuffer = 8609d21e TransferBufferMDL= : c1 UrbLink = RequestTypeReservedBits = Request = 0003 Value = 3000 Index = 00fe [46038 ms] UsbSnoop - MyInternalIOCTLCompletion(f79b7db0) : fido=, Irp=8 58f2938, Context=858ccea0, IRQL=2 [46038 ms] <<< URB 26 coming back <<< -- URB_FUNCTION_CONTROL_TRANSFER: PipeHandle = 86239260 TransferFlags= 000a (USBD_TRANSFER_DIRECTION_OUT, USBD_SHORT_TRANS FER_OK) TransferBufferLength = 0001 TransferBuffer = 8609d21e TransferBufferMDL= 855f7760 UrbLink = SetupPacket = : 40 03 00 30 fe 00 01 00 [46038 ms] >>> URB 27 going down >>> -- URB_FUNCTION_VENDOR_DEVICE: TransferFlags = 0001 (USBD_TRANSFER_DIRECTION_IN, ~USBD_SHORT_TRA NSFER_OK) TransferBufferLength = 0001 TransferBuffer = 8609d21e TransferBufferMDL= UrbLink = RequestTypeReservedBits = Request = 0002 Value = 3000 Index = 0100 [46040 ms] UsbSnoop - MyInternalIOCTLCompletion(f79b7db0) : fido=, Irp=8 58f2938, Context=86366778, IRQL=2 [46040 ms] <<< URB 27 coming back <<< -- URB_FUNCTION_CONTROL_TRANSFER: PipeHandle = 86239260 TransferFlags= 000b (USBD_TRANSFER_DIRECTION_IN, USBD_SHORT_TRANSF ER_OK) TransferBufferLength = 0001 TransferBuffer = 8609d21e TransferBufferMDL= 855f7760 : 7c UrbLink = SetupPacket = : c0 02 00 30 00 01 01 00 Note: In log 2 & 3, "Request" parameter value is different from log 1. regards, Akihiro
Re: [PATCH 3/4] tc90522: add driver for Toshiba TC90522 quad demodulator
On 2014年08月16日 01:09, Mauro Carvalho Chehab wrote: >> +++ b/drivers/media/dvb-frontends/tc90522.c >> +static int tc90522s_get_frontend(struct dvb_frontend *fe) >> +{ . >> +c->delivery_system = SYS_ISDBS; >> +c->symbol_rate = 2886; > > Hmm... symbol rate is fixed? That looks weird on my eyes. Though I may be totally wrong, I set it to 28.86M because ARIB STD B20 (section 2.3) defines the speed of the modulated carrier signal be 28.860Mbaud. But carrier is splitted into 48 slots and they are assigned to multiple TS's, which are modulated with (one of or a combination of) either 8PSK or QPSK or BPSK. Dummy slots are included in the case of QPSK/BPSK. The output TS is chosen by TS_ID. Should I set c->symbol_rate to (# of slots assigned to the output TS)/48 * 28.86Mbaud ? >> +static int tc90522t_get_frontend(struct dvb_frontend *fe) >> +{ >> +static const fe_transmit_mode_t tm_conv[] = { >> +TRANSMISSION_MODE_2K, >> +TRANSMISSION_MODE_4K, >> +TRANSMISSION_MODE_8K, 0 > > Better to put the final 0 on a separate line, to be consistent. > > I would also move those tables out of get_frontend, as this improves > readability. Do "those tables" include generally other "static const" tables in other functions? there are lots of small, function-scoped const tables which define register values and just used in one function, like the following ones. >> +static int tc90522_set_frontend(struct dvb_frontend *fe) >> +{ >> +static const struct reg_val reset_sat = { 0x03, 0x01 }; >> +static const struct reg_val reset_ter = { 0x01, 0x40 }; >> +state = fe->demodulator_priv; >> + >> +if (fe->ops.tuner_ops.set_params) >> +ret = fe->ops.tuner_ops.set_params(fe); > > Hmm... I'm not seeing any part of the driver parsing the ISDB > parameters (except for the frequency). (and TS_ID, layers). As this demod looks into TMCC and adjusts lots of parameters automatically, with preset defaults (for Japanese ISDB-S/T). Thus the frequency is the minimum and sufficient parameter to demodulate. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] pt3: add support for Earthsoft PT3 ISDB-S/T receiver card
On 2014年08月16日 01:17, Mauro Carvalho Chehab wrote: .. >> +++ b/drivers/media/pci/pt3/pt3.c .. >> +static int pt3_fetch_thread(void *data) >> +{ >> +struct pt3_adapter *adap = data; >> + >> +#define PT3_FETCH_DELAY (10 * 1000) >> +#define PT3_INITIAL_DISCARD_BUFS 4 >> + >> +pt3_init_dmabuf(adap); >> +adap->num_discard = PT3_INITIAL_DISCARD_BUFS; >> + >> +dev_dbg(adap->dvb_adap.device, >> +"PT3: [%s] started.\n", adap->thread->comm); >> +while (!kthread_should_stop()) { >> +pt3_proc_dma(adap); >> +usleep_range(PT3_FETCH_DELAY, PT3_FETCH_DELAY + 2000); >> +} >> +dev_dbg(adap->dvb_adap.device, >> +"PT3: [%s] exited.\n", adap->thread->comm); >> +adap->thread = NULL; >> +return 0; >> +} > > Why do you need a thread here? Having a thread requires some special > care, as you need to delete it before suspend, restore at resume > (if active) and be sure that it was killed at device removal. > > I'm not seeing any of those things on this driver. PT3 is a dumb device that lacks interrupt, so the driver has to poll the DMA buffers regularly to know if a DMA has finished. I forgot to clean up the thread in remove() and will add it in the v2. As with suspend/resume, I can add a check of freezing() like in dvb_frontend.c::dvb_frontend_thread(), but if I remember right, I had read before that someone pointed out that the suspend/resume of DVB is not supported by DVB core, and not many of other drivers seem to support it either, so I thought I could omit this feature and rely on module re-probing. Should I implement power management ops and set them to .driver.pm? -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] mxl301rf: add driver for MaxLinear MxL301RF OFDM tuner
Hi Mauro, thanks for the review. I will update the patch and post it later. Before doing so, I'd like to ask some questions. >> +++ b/drivers/media/tuners/mxl301rf.c >> +/* *strength : [1/1000 dBm] */ >> +static int mxl301rf_get_rf_strength(struct dvb_frontend *fe, u16 *strength) >> +{ ... > Please implement it using DVBv5 API, e. g. using dBm scale. Do you mean that fe->tuner_ops.get_rf_strength() is deprecated and should not be used? And as you pointed me out in the review of tc90522, > The better is likely to add a new ops to get rf strength in dBm as s64, > just like the way we use it on DVBv5. should I add a new tuner_ops to the DVB core like get_rf_strength_v5(fe,&s64)? Or should mxl301rf provide raw u16 value and tc90522 convert it to s64 like in dvb-frontends/dib7000p.c? >> +static int mxl301rf_set_params(struct dvb_frontend *fe) >> +{ >> + >> +/* spur shift function (for analog) */ >> +for (i = 0; i < ARRAY_SIZE(shf_tab); i++) { >> +if (freq >= (shf_tab[i].freq - shf_tab[i].ofst_th) * 1000 && >> +freq <= (shf_tab[i].freq + shf_tab[i].ofst_th) * 1000) { >> +tune0[2 * 5 + 1] = shf_tab[i].shf_val; >> +tune0[2 * 6 + 1] = 0xa0 | shf_tab[i].shf_dir; >> +break; >> +} >> +} > > Hmm... are you using a table to match the channels? If so, don't do that. > Instead, just calculate the dividers based on the given frequency. mxl301rf requires to set frequency instead of the dividers, as in the following section. >> +/* convert freq to 10.6 fixed point float [MHz] */ >> +f = freq / 100; >> +tmp = freq % 100; >> +div = 100; >> +for (i = 0; i < 6; i++) { >> +f <<= 1; >> +div >>= 1; >> +if (tmp > div) { >> +tmp -= div; >> +f |= 1; >> +} >> +} >> +if (tmp > 7812) >> +f++; >> +tune1[2 * 0 + 1] = f & 0xff; >> +tune1[2 * 1 + 1] = f >> 8; >> +ret = data_write(state, tune1, sizeof(tune1)); >> +if (ret < 0) >> +goto failed; shf_tab[] holds another parameters for another purpose ("spur shift"?), whose values depend on the range of the input frequency. This table is ported from the reference "SDK" source of PT3, and no further info is disclosed. I made an experiment that removes the code to set those parameters and it worked fine without problems in my environment, but I kept the code since I don't know what those parameter exactly mean. >> +}, >> + >> +.release = mxl301rf_release, >> +.init = mxl301rf_init, >> +.sleep = mxl301rf_sleep, >> + >> +.set_params = mxl301rf_set_params, >> +.get_status = mxl301rf_get_status, >> +.get_rf_strength = mxl301rf_get_rf_strength, > > Isn't it capable of providing more stats? > I can add .get_frequency() or .get_bandwidth(), but those are not used in DVB core and frontends can get those info from its property cache. Should those trivial funcs be implemented? (As with .get_{if_freq,afc}(), necessary info is not disclosed.) regards, akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/5] dvb-core: add a new tuner ops to dvb_frontend for APIv5
moikka, thanks for the comment. > I have feeling DVBv5 API is aimed to transfer data via property cached. > I haven't done much driver for DVBv5 statistics, but recently I > implemented CNR (DVBv5 stats) to Si2168 driver and it just writes all > the values directly to property cache. I expect RF strength (RSSI) is > just similar. Currently, the demod of PT3 card (tc90522) gets RSSI data from the connected tuner (mxl301rf) via tuner_ops.get_signal_strength_dbm() and sets property cache in fe->ops.get_frontend() (which is called before returning property cache value by dvb_frontend_ioctl_properties()). If the tuner driver should set property cache directly, when is the right timing to do so? In fe->ops.tuner_ops.get_status() ? or in the old fe->ops.tuner_ops.get_signal_strength()? or Should I change get_signal_strength_dbm(fe, s64 *) to update_signal_strength(fe) and let the tuner driver set property cache there? -- akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/5] dvb-core: add a new tuner ops to dvb_frontend for APIv5
moikka, > Start polling thread, which polls once per 2 sec or so, which reads RSSI > and writes value to struct dtv_frontend_properties. That it is, in my > understanding. Same for all those DVBv5 stats. Mauro knows better as he > designed that functionality. I understand that RSSI property should be set directly in the tuner driver, but I'm afraid that creating a kthread just for updating RSSI would be overkill and complicate matters. Would you give me an advice? >> Mauro regards, akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/5] tc90522: add driver for Toshiba TC90522 quad demodulator
Hi Matthias, thanks for the comment. > it sounds wrong to export a second function besides tc90522_attach. > This way there is a hard dependency of the bridge driver to the demod > driver. > In this case it is the only possible demod, but in general it violates > the design of demod drivers and their connection to bridge drivers. I agree. I missed that point. > > si2168_probe at least has a solution for this: > Write the pointer to the new i2c adapter into location stored in "struct > i2c_adapter **" in the config structure. I'll look into the si2168 code and update tc90522 in v3. regards, akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/5] tc90522: add driver for Toshiba TC90522 quad demodulator
Hi, > Also, I would like to see all new drivers (demod and tuner) implemented > as a standard kernel I2C drivers (or any other bus). I have converted > already quite many drivers, si2168, si2157, m88ds3103, m88ts2022, > it913x, tda18212, ... I wrote the code in the old style using dvb_attach() because (I felt) it is simpler than using i2c_new_device() by introducing new i2c-related data structures, registering to both dvb and i2c, without any new practical features that i2c client provides. But if the use of dvb_attach() is (almost) deprecated and i2c client driver is the standard/prefered way, I'll convert my code. regards, akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/5] tc90522: add driver for Toshiba TC90522 quad demodulator
Moi! > Yes, using the I2C binding way provides a better decoupling than using the > legacy way. The current dvb_attach() macros are hacks that were created > by the time where the I2C standard bind didn't work with DVB. I understand. I converted my code to use i2c binding model, but I'm uncertain about a few things. 1. How to load the modules of i2c driver? Currently I use request_module()/module_put() like an example (ddbrige-core.c) from Antti does, but I'd prefer implicit module loading/ref-counting like in dvb_attach() if it exists. 2. Is there a standard way to pass around dvb_frontend*, i2c_client*, regmap* between bridge(dvb_adapter) and demod/tuner drivers? Currently I use config structure for the purpose, which is set to dev.platform_data (via i2c_board_info/i2c_new_device()) or dev.driver_data (via i2c_{get,set}_clientdata()), but using config as both IN/OUT looks a bit hacky. 3. Should I also use RegMap API for register access? I tried using it but gave up, because it does not fit well to one of my use-case, where (only) reads must be done via 0xfb register, like READ(reg, buf, len) -> [addr/w, 0xfb, reg], [addr/r, buf[0]...], WRITE(reg, buf, len) -> [addr/w, reg, buf[0]...], and regmap looked to me overkill for 8bit-reg, 8bit-val cases and did not simplify the code. so I'd like to go without RegMap if possible, since I'm already puzzled enough by I2C binding, regmap, clock source, as well as dvb-core, PCI ;) regards, Akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/5] dvb-core: add a new tuner ops to dvb_frontend for APIv5
Moikka!, thanks for the comments and advices. I had been updating my code and during that, I also found that updating property cache in tuner_ops.get_signal_strength() was simple and (seemed to me) better than using a kthread, so the current implementation (under testing) is just like what Mauro proposed, but, > In time: not implementing its own thread has one drawback: the driver needs > to check if the minimal time needed to get a new stats were already archived. since I don't know the minimal time and whether there's a limit in the first place, I'd like to let users take the responsibility. >> ... I simply don't understand why you want hook that RF strength call >> via demod? The frontend cache is shared between demod and tuner. We use >> it for tuner drivr as well demod driver. Let the tuner driver make RSSI >> calculation independently without any unneeded relation to demod driver. I think the main reason for the hook is because the dvb-core calls ops.get_frontend() everytime before reading of any property cache, so it is already a nice place to trigger property updates, and reading any property involves demod (FE as a whole) anyway. regards, akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/5] tc90522: add driver for Toshiba TC90522 quad demodulator
moikka!, > Basically it is 2 functions, af9035_add_i2c_dev() and af9035_del_i2c_dev() I used request_module()/try_module_get()/module_put() just like the above example (and bridge-core.c). It works, but when I unload bridge driver(earth_pt3), its demod and tuner modules stay loaded, with the refcount of 0. Is it ok that the auto loaded modules remain with 0 ref count? > Yet, using config to OUT seems to be bit hacky for my eyes too. I though > replacing all OUT with ops when converted af9033 driver. Currently > caller fills struct af9033_ops and then af9033 I2C probe populates ops. > See that patch: > https://patchwork.linuxtv.org/patch/25746/ > > Does this kind of ops sounds any better? Do you mean using ops in struct config? if so, I don't find much difference with the current situation where demod/tuner probe() sets dvb_frontend* to config->fe. > I quickly overlooked that demod driver and one which looked crazy was > LNA stuff. You implement set_lna callback in demod, but it is then > passed back to PCI driver using frontend callback. Is there some reason > you hooked it via demod? You could implement set_lna in PCI driver too. Stupidly I forgot that FE's ops can be set from the PCI driver. I will remove those callbacks and set the corresponding ops instead. regards, akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/4] mxl301rf: add driver for MaxLinear MxL301RF OFDM tuner
Moi!, thanks for the review. >> +static int reg_read(struct mxl301rf_state *state, u8 reg, u8 *val) ... >> +ret = i2c_transfer(state->i2c->adapter, msgs, ARRAY_SIZE(msgs)); >> +if (ret >= 0 && ret < ARRAY_SIZE(msgs)) >> +ret = -EIO; >> +return (ret == ARRAY_SIZE(msgs)) ? 0 : ret; >> +} > > Could you really implement that as a I2C write with STOP and I2C read > with STOP. I don't like you abuse, without any good reason, > i2c_transfer() with REPEATED START even we know chip does not do it. > > You should use > i2c_master_send() > i2c_master_recv() Though it's woking with this REPEATED_START style reads, (and my reference win driver also uses REPEATED_START), I'll incorporate this in the next version. >> +static int mxl301rf_get_status(struct dvb_frontend *fe, u32 *status) . > And whole function is quite useless in any case. It was aimed for analog > radio driver originally, where was audio demod integrated. We usually > just program tuner first, then demod, without waiting tuner lock, as > tuner locks practically immediately to given freq. It is demod which > locking then has any sense. > > Tuner PLL lock bits could be interesting only when you want to test if > you are in a frequency whole tuner is able to receive. Some corner case > when tuner is driven over its limits to see if it locks or not. I understand. I'll remove .get_status(). >> +static int mxl301rf_init(struct dvb_frontend *fe) ... >> +/* tune to the initial freq */ ... > This looks odd. Why it is tuned here to some freq? What happens if you > don't do it and it will be tuned to requested freq. Sometimes that kind > of things are used to initialize badly written driven... In a PT3 board, mxl301rf is packaged into a canned tuner module (Sharp VA4M6JC2103) with another mxl301rf and two qm1d1c0042's. A reference win driver says that it is to avoid "interference" between mxl301rf and qm1d1c0042, so I added a config parameter of initial freq. I could have moved those initial tunings to the PCI driver, but I don't know if it is a corner case that applys just to PT3. I must admit that my code is written pretty badly, but it is partly;) because the available/disclosed information is very limited to the reference win driver kit, it hides lots of register settings including those for init/config, and is badly written not separating demod/tuner modules well. >> +static const struct dvb_tuner_ops mxl301rf_ops = { .. >> +.init = mxl301rf_init, >> +.sleep = mxl301rf_sleep, >> + >> +.set_params = mxl301rf_set_params, >> +.get_status = mxl301rf_get_status, >> +.get_rf_strength = mxl301rf_get_rf_strength, > > get IF frequency is missing. That is tuner using IF so you will need to > know IF in order to get demod working. As the product guide of TC90522 says it can accept 3-6MHz low IF or 44/57MHz direct IF, so there must be the register setting to select/set/get one of these configs. But I don't have the data sheets of mxl301rf, and cannot know which demod/tuner reigsters are set during init/config phase (as I said above it's not disclosed), so I don't know the registers to set/get IF. The tuner/demod drivers I wrote are certainly imcomplete ones that lack init/config of the chips, but currently they are used just by PT3 and when someone gets to use them in other products, I expect that [s]he would have more info and update my code. regards, akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] dvb-core: add template code for i2c binding model
Hi, thank you for the comment. I understood the naming conventions you mentioned, and I'll update them in the next version. >> diff --git a/drivers/media/dvb-core/dvb_i2c.c >> b/drivers/media/dvb-core/dvb_i2c.c >> new file mode 100644 >> index 000..4ea4e5e >> --- /dev/null >> +++ b/drivers/media/dvb-core/dvb_i2c.c >> +static struct i2c_client * >> +dvb_i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info *info, >> + const unsigned short *probe_addrs) >> +{ >> +struct i2c_client *cl; >> + >> +request_module(I2C_MODULE_PREFIX "%s", info->type); >> +/* Create the i2c client */ >> +if (info->addr == 0 && probe_addrs) >> +cl = i2c_new_probed_device(adap, info, probe_addrs, NULL); >> +else >> +cl = i2c_new_device(adap, info); >> +if (!cl || !cl->dev.driver) >> +return NULL; >> +return cl; > > The best would be to also register the device with the media controller, > if CONFIG_MEDIA_CONTROLLER is defined, just like v4l2_i2c_subdev_init() > does. I'll comment to your patch on this. > I would also try to use similar names for the function calls to the ones > that the v4l subsystem uses for subdevices. So the name should be dvb_i2c_new_subdev()? I am a bit worried that it would be rather confusing because this func is different from v4l2_i2c_new_subdev() in that it does not return "struct dvb_frontend *", requires info.platform_data parameter, and is a static/internal function. One more thing that I'm wondering about is whether we should add fe->demod_i2c_bus to support tuner devices on a (dedicated) i2c bus hosted on a demod device. I guess that this structure is pretty common (to reduce noise), and this removes i2c_adapter from "out" structure and confines the usage of "out" structure to just device-specific ones. -- akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] dvb-core: add template code for i2c binding model
> The only thing I noticed is that it is causing some warnings at > dmesg about trying to create already created sysfs nodes, when the > driver is removed/reinserted. > > Probably, the remove callback is called too soon or too late. I don't have any warnings in syslog when reinserting earth-pt3 + tc90522, but I'll look into it. > -struct dvb_frontend *mb86a20s_attach(const struct mb86a20s_config *config, > - struct i2c_adapter *i2c) > +static int mb86a20s_probe(struct i2c_client *i2c, > + const struct i2c_device_id *id) > { > + struct dvb_frontend *fe; > struct mb86a20s_state *state; > u8 rev; > > dev_dbg(&i2c->dev, "%s called.\n", __func__); > > - /* allocate memory for the internal state */ > - state = kzalloc(sizeof(struct mb86a20s_state), GFP_KERNEL); > - if (state == NULL) { > - dev_err(&i2c->dev, > - "%s: unable to allocate memory for state\n", __func__); > - goto error; > - } > + fe = i2c_get_clientdata(i2c); > + state = fe->demodulator_priv; > > /* setup the state */ > - state->config = config; > + memcpy(&state->config, i2c->dev.platform_data, sizeof(state->config)); > state->i2c = i2c; > > /* create dvb_frontend */ > - memcpy(&state->frontend.ops, &mb86a20s_ops, > + memcpy(&fe->ops, &mb86a20s_ops, > sizeof(struct dvb_frontend_ops)); btw, we can go with "mb86a20s_param = { .ops.fe_ops = &mb86a20s_ops,}" insead. -- regards, akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] mb86a20s: convert it to I2C binding model
moikka, On 2015年01月02日 06:30, Antti Palosaari wrote: > I am on holiday trip now. But generally speaking I would like to separate all > drivers from the interfaces. That means for example I2C tuner driver is just > a I2C driver and nothing more - no relations to DVB nor V4L API. That is > something I said many times earlier too, but for my taste drivers should be > agnostics to APIs. I can't yet fully understand what API agnostic driver means, but if it is like the implementation of the current i2c tuner/demod drivers, each adapter drivers must (re-)implement common codes like module loading/ref-counting and demod/tuner drivers must implement dvb-core related initializations on their own. In addition, they may take different ways in how the data like "fe", config data and other device specific output parameters are passed around between adapters and demod/tuners. I thought it would be better to intergrate those common codes into dvb_core to ease maintainance and let each driver concentrate on driver-specific things. And don't the current i2c tuner drivers already depend on dvb_core practically by implementing dvb_tuner_ops functions and filling in their pointers into fe->ops.tuner_ops? regards, akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] dvb core: add media controller support for the demod
On 2015年01月02日 00:51, Mauro Carvalho Chehab wrote: > /* > @@ -416,6 +418,11 @@ struct dvb_frontend { > struct dvb_frontend_ops ops; > struct dvb_adapter *dvb; > struct i2c_client *fe_cl; > +#if defined(CONFIG_MEDIA_CONTROLLER) > + struct media_device *mdev; > + struct media_entity demod_entity; > +#endif > + I understood that this patch was invalidated by the updated patch series: "dvb core: add basic suuport for the media controller", and now the demod_entity is registered in dvbdev.c::dvb_register_device() via dvb_frontend_register(). Is that right? And if so, Shouldn't only the (tuner) subdevices be registered separately in dvb_i2c_attach_tuner(), instead of dvb_i2c_attach_fe()? (and it would be simpler if "mdev" can be accessed like dvb_fe_get_mdev() {return fepriv->dvbdev->mdev;}, instead of having a cached value in dvb_frontend.) sorry if I'm totally wrong, I don't have an experience with media controller API. regards, akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] dvb core: add media controller support for the demod
On 2015年01月06日 03:29, Mauro Carvalho Chehab wrote: > Em Mon, 05 Jan 2015 23:36:00 +0900 >> And if so, >> Shouldn't only the (tuner) subdevices be registered separately >> in dvb_i2c_attach_tuner(), instead of dvb_i2c_attach_fe()? > > No, it seems better to let dmxdev to register. That means that even > the non-converted I2C drivers, plus the non-I2C drivers may benefit > from the Media controller as well. I guess you meant dvbdev instead of dmxdev, but still I'm afraid that media_entity for a tuner module is not registered like FE(demod),dmxdev,net,ca, because dvb_frontend_register() registers just a demod (or a whole FE module) for now. Tuner subdevices don't have their own device nodes to be passed to dvb_register_media_device() in the first place, So I think we need to define MEDIA_ENT_T_DVB_SUBDEV{,_TUNER} and register a media_entity of type:MEDIA_ENT_T_DVB_SUBDEV_TUNER, if a separate tuner chip is used. (with {major,minor} of the parent FE dev?) If we can distinguish if a separate tuner subdev is used or not by looking into "fe", then we might be able to register the subdev from within dvb_register_[media_]device(). But if we cannot know from "fe", dvb_i2c_attach_tuner() looks to be a good place. Further, when a DTV board has multiple tuners and/or multiple demods and the link between them can be dynamically configurable, (via media controller API or a new DVB ioctl?), then a tuner subdev can be unused and connected to no demod, so we cannot identify all tuner subdevs from the demod ("fe") anyway. > Yeah, we could map this way, but that would require to add an extra > parameter to the fe register function, with has already too much > parameters. So, as it already uses an struct to pass parameters into > it, I decided to just re-use it. if we add dvb_adapter.mdev and a user sets the pointer, then we can retrieve mdev in all the registering functions (dvb_register_frontend, dvb_{dmxdev,net,ca_en50221}_init), as all of them take a dvb_adapter parameter. or is it possible that the FE,demux,net,ca under one DVB adapter belong to different mdev's and each of them must keep its parent mdev separately? regards, akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] dvb-core: add template code for i2c binding model
Moikka, thank you for the comment. On 2015年01月14日 03:54, Antti Palosaari wrote: >> --- a/drivers/media/dvb-core/dvb_frontend.h >> +++ b/drivers/media/dvb-core/dvb_frontend.h >> @@ -415,6 +415,7 @@ struct dtv_frontend_properties { >> struct dvb_frontend { >> struct dvb_frontend_ops ops; >> struct dvb_adapter *dvb; >> +struct i2c_client *fe_cl; > > IMHO that is ugly as hell. You should not add any hardware/driver things > to DVB frontend, even more bad this adds I2C dependency to DVB frontend, > how about some other busses... DVB frontend is *logical* entity > representing the DVB device to system. All hardware specific things > belongs to drivers - not the frontend at all. Currently, the i2c_client pointer is distributed/copied among each demod drivers plus adapter drivers, so I thought it would be better to unify them into one place, but since there seems to be no good (common) place to store it, I'll consider your advice and remove this fe_cl member. Regards, akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] dvb-core: add template code for i2c binding model
Moikka, thank you for the comment. (and sorry for the late reply. I was off from work until recently.) > Basically the issue is registering I2C driver and after that reference > counting it as there is dvb frontend ops which are called. In current DVB drivers, "struct dvb_frontend"s can be alloc'ed in two places, in FE driver itself and in the client(adapter) driver, but the patch alloc's the struct dvb_frontned in the probe() helper function, so you cannot access frontend ops before registering the driver module. The probe() helper function ref-counts the driver module as well, so from clients' point of views, the driver registering and the ref-counting are done at the same time. > > Did you make study how the others have resolved that issue? Could you list > already used methods? I just read through the V4L2-core i2c subdev code and did not studied much on other modules, tbh:P Regards, akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Full DVB driver package for Earthsoft PT3 (ISDB-S/T) cards
Hi Bud, It seems that the tuner modules use fe->ops.write() for i2c communications, which is set by parent module. but I'm afraid fe->ops.write() is not for the purpose. Shouldn't they use i2c_adapters passed from _attach() etc. instead? regards, akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ISDB caption support
> I realize captions is an application-layer function, and intend to work > with the CCExtractor team. Do any other applications already have ISDB > caption support? there's a mplayer patch for subtitle support: https://github.com/0p1pp1/mplayer/commit/6debc831d34cad98d1b251920fbdb48f74a880df It translates subtitle stream PES to ASS, but is is for ISDB-T/Japan. Subtitling in ISDB-T depends heavily on the control sequences of the original character encoding (ARIB STD-B24), so I'm afraid that (at least) PES format is very different in ISDB-Tb. regards, akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Random memory corruption of fe[1]->dvb pointer
> So at first it would be nice if someone could confirm my findings. > Applying the same kind of code like my patch and unplug something that > uses the affected frontend should be enough. I tried that for tc90522, and I could remove earth-pt3 (which uses tc90522), tc90522 and tuner modules without any problem, although earth-pt3 is a pci driver and does not use dvb-usb-v2. >From your log(?) output, I guess that rtl28xxu_exit() removed the attached demod module (mn88472) and thus free'ed fe BEFORE calling dvb_usbv2_exit(), from where dvb_unregister_frontend(fe) is called. I think that the demod i2c device is removed automatically by dvb_usbv2_i2c_exit() in dvb_usbv2_exit(), if you registered the demod i2c device, and your adapter/bridge driver should not try to remove it. regards, Akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] dvb-core: add template code for i2c binding model
ping. we don't need this kind of feature in the dvb-core? -- akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/4] tc90522: add driver for Toshiba TC90522 quad demodulator
Hi, On 2014年09月24日 05:07, Mauro Carvalho Chehab wrote: > I applied this series, as we're discussing it already for a long time, > and it seems in a good shape... thanks for your reviews and advices. >> +static int tc90522s_read_status(struct dvb_frontend *fe, fe_status_t >> *status) ... >> +if (reg & 0x60) /* carrier? */ >> +return 0; > > Sure about that? Wouldn't it be, instead, reg & 0x60 == 0x60? Yes, I'm pretty sure about that. The register indicates errors in the various demod stages, and if all go well, the reg should be 0. >> +static int tc90522t_read_status(struct dvb_frontend *fe, fe_status_t >> *status) .. > The entire series of checks above seems wrong on my eyes too. > > For example, if reg = 0x20 or 0x40 or 0x80 or ..., it will return > FE_HAS_LOCK. This register 0x96 should indicates "lock" status for each layers, and since layer config can vary in ISDB-T, the driver checks that any of the three bits is set, for faster lock detection. and the register 0x80 is the same kind of the one in the above ISDB-S case. > PS.: could you also test (and send us patches as needed) for ISDB-S > support at libdvbv5 and dvbv5-utils[1]? I'll have a try. regards, akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] tc90522: declare tc90522_functionality as static
Sorry to bother you with those build warnings. Somehow I didn't see them when I built the modules myself on 64bit box. maybe since I forgot to test all-y config (and 32bit build), the gcc cmd did not include [-Wmissing-prototypes]. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/4] tc90522: add driver for Toshiba TC90522 quad demodulator
> Btw, please check if your driver is handling it well, as the valid > values for interleave are 0, 1, 2, 4 (actually, dib8000 also > supports interleaving equal to 8, if sound_broadcast). I have no info on how to set time interleaving parameters, although I can read them as a part of TMCC info. I guess I can also set them in the same registers, but I'm not sure. Since the demod is intelligent and automatically reads TMCC and sets the related parameters accordingly, I think it is safe to ignore user settings and let the demod set it. If someone would get more info in the future, [s]he could set those parameters in advance and make a lock faster. regards, akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/4] tc90522: add driver for Toshiba TC90522 quad demodulator
> And TMCC returns it as 0, 1, 2, 3, 4, instead of 0, 1, 2, 4, 8. According to ARIB STD B31 3.15.6.7 (table 3.30), The value is encoded and depends on the "mode". so demods have to encode/decode anyway. > AFAIKT, that doesn't work in Sound Broadcast mode, as the demod won't > be able to read the TMCC (at least on most demods - don't have any > details about tc90522). I forgat ISDB-Tsb;P, as in Japan terrestrial digital sound broadcast was cancelled in 2011 and it was unpopular. -- akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] qm1d1c0042: fix compilation on 32 bits
> - b = (((s64) freq) << 20) / state->cfg.xtal_freq - (((s64) a) << 20); > + b = (s32)div64_s64(((s64) freq) << 20, > +state->cfg.xtal_freq - (((s64) a) << 20)); > + I'm afraid it should be like the following. > + b = (s32)(div64_s64(((s64) freq) << 20, state->cfg.xtal_freq) > + - (((s64) a) << 20)); regads, akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/4] tc90522: add driver for Toshiba TC90522 quad demodulator
> Ok, but see the definition at the API spec: > > > http://linuxtv.org/downloads/v4l-dvb-apis/FE_GET_SET_PROPERTY.html#isdb-hierq-layers > > Basically, we use 0, 1, 2, 4 no matter what mode, e. g. I understand. (though I wonder why "4" is assigned instead of "3" when it is not a flag bit that can be OR'ed and a kind of "raw" values are used anyway). > Ah, I wasn't aware that ISDB-Tsb was cancelled in Japan. The band used by ISDB-Tsb is now assigned to ISDB-Tmm (which is also very unpopular), and there is a plan to use ISDB-Tsb in another freq band in the future, but no concrete one yet. -- akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pt3: fix DTV FE I2C driver load error paths
> Maybe that is proper fix. I didn't test it. I had some tests with/without inserting a deliberate error return from dvb_register_frontend() and with/without CONFIG_MODULES option, and the all combinations seem to have worked fine. kiitos, Antti and Randy. -- akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/11] tc90522 is a client
(some Cc:'s removed, to exclude those who are not directly involved) It seems that I should make some counterarguments. On 2014年10月05日 21:39, AreMa Inc. wrote: > We started developing and publishing PT3 drivers (chardev and DVB versions) > on 2013 and have been submitting the patches to this community since then. > We were waiting for reviews. you haven't annouced any intent to merge this github project to the linux kernel until this April. It also has not acquired so many users that a bug in parallel streaming had been left unnoticed until just recently. So I disagree that your project is > a de facto standard, > well tested without any major/minor issues > reported , but it's a small thing anyway, comparted to the other claims by you. I read your first post this April in the ML, and exchanged some private emails after sending my reply to the ML, about the reviews on the original post. I waited 3 months, expecting you revise and re-post the patch, but you didn't. As I knew that your code did not well conform to the current DVB driver model/style nor kernel patch submission rules, I sent a private email that I was afraid that the patch you sent to the ML would not be accepted if left as it is, and asked if you would continue revising it, and if not, I would take over the submitting of PT3 driver. But you replied that you were reluctant to change the original code and clealy rejected some of the reviews incl. separating of the patch. I still waited one more week expecting you to revise the patch or at least express some counterarguments on the ML, but no responses. Thus I figured that you don't have a will to revise the patch anymore and gave up merging it, so I post my code with reference to your patch. But you didn't repsoned anything, again. > However this July, a man named Tsukada, who has been annoying us since > the beginning of development (we invited him to merge/join the project, > in other words, opted him to be co-author), interrupted our submission > and started > speaking ill of us that we didn't want to split the driver and stopped > the development, etc. plz don't lie. you just said that you would accept patches to your origianl code base at github if I would sent those. Never mentioned joining the project or co-writing. and I never spoke ill of you. I just pointed out the reason why I took over the driver, which is just same as you mentioned in the above paragraph. If you had an objection to my patch, Why you didn't express objections for well more than one month after my first post? Why you didn't respond to the first review to your patch for more than 3 months? Did you remember that you even "DECLINED" separting your patch later in September? (see 1409966878-22627-1-git-send-email-knightri...@are.ma in the ML) As I said in the private email to you, all those things made me believe that you would not update your patch to conform to the DVB driver model and thus it would not be accepted in the future, and that you had admitted my patch since you did not express any objections. -- akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] v4l-utils/dvbv5-scan: add support for ISDB-S scanning
On 2014年10月09日 01:04, Mauro Carvalho Chehab wrote: >> @@ -251,6 +251,16 @@ static int run_scan(struct arguments *args, >> if (dvb_retrieve_entry_prop(entry, DTV_POLARIZATION, &pol)) >> pol = POLARIZATION_OFF; >> >> +if (parms->current_sys == SYS_ISDBS) { >> +uint32_t tsid = 0; >> + >> +dvb_store_entry_prop(entry, DTV_POLARIZATION, >> POLARIZATION_R); >> + >> +dvb_retrieve_entry_prop(entry, DTV_STREAM_ID, &tsid); >> +if (!dvb_new_ts_is_needed(dvb_file->first_entry, entry, >> + freq, shift, tsid)) >> +continue; > > This is likely needed for DVB-T2 and DVB-S2 too. Should we compare channel entries by (freq, stream_id, polarization) triplet instead of by the current (freq, polarization) or (freq, stream_id)? >> @@ -258,6 +268,10 @@ static int run_scan(struct arguments *args, >> count++; >> dvb_log("Scanning frequency #%d %d", count, freq); >> >> +if (!args->lnb_name && entry->lnb && >> +(!parms->lnb || strcasecmp(entry->lnb, parms->lnb->alias))) > > Shouldn't it be: !strcasecmp(entry->lnb, parms->lnb->alias)? Or maybe just > remove this test. I want to update parms->lnb (which was set from the prev entry) only if it differs from entry->lnb (current one), and don't want to linear-search all LNB types for every entries, as lots of entries are expected to have the same LNB types. -- akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] v4l-utils:libdvbv5,dvb: add basic support for ISDB-S
Hi, Mauro, thanks for the review. I'll update the patch soon. And I wrote some comments and questions as a reply to your each review mail. On 2014年10月09日 01:22, Mauro Carvalho Chehab wrote: > Yeah, it will likely require a table just like the one we've added > for EN 300 468 specific charset with the euro sign. > > There are some patches for charset decoding like this one: > > http://marc.info/?l=mplayer-dev-eng&m=125642040004816 I already looked over other examples in some Japan local patches to the applications like mplayer,gstreamer, mythtv..., ( https://github.com/0p1pp1?tab=repositories https://github.com/takaakis62/mythtv_isdb ) so I'm going to reuse those code. regards, akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dvb:tc90522: bugfix of always-false expression
>> Reported by David Binderman > > ^^ See Documentation/SubmittingPatches Though I knew that Reported-by: tag should not be used, I wrote it just to express my appreciation for his report, and did not mean to attach the tag. But I admit that it is confusing, so I'd like to beg Mauro to do me the kindness to delete the line when this patch is committed. (or I'll re-send the patch if it is necessary.) regards, akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dvb-core: set default properties of ISDB-S
On 2014年10月27日 01:33, Antti Palosaari wrote: > You should calculate bw too as tuners needs set filters according to > used channel bw. You could calculate nominal needed be using formula: > bandwidth = roll-off factor * symbol rate I'll include it in the next version. thx -- Akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/7] v4l-utils/libdvbv5: add support for ISDB-S scanning
On 2014年10月27日 23:46, Mauro Carvalho Chehab wrote: > Always right circular polarization? I guess I read something at the specs > sometime ago about left polarization too. Not sure if this is actually used. Currently all transponders of ISDB-S use right polarization. but I modifed this part and removed POLARIZATION prop from ISDB-S in v3. -- Akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/7] v4l-utils/libdvbv5: add gconv module for the text translation of ISDB-S/T.
> Using my Fedora package maintainer's hat, I would only enable this > on a patch that would also be packaging the gconf module as a > separate subpackage, as this would make easier to deprecate it > if/when this gets merged at gconv's upstream. I'm afraid that gconv upstream won't merge those application domain specific encodings, as I posted an inquiry about the acceptance of such gconv modules to the glibc ML's, but got no response until now. (and it seems that the current glibc does not include app specific char-encodings). -- Akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 6/7] v4l-utils/libdvbv5: don't discard config-supplied parameters
I withdrew this patch as it's meaningless. Originally I mis-understood that when dvbv5-scan was run with "-G" option, the props in channel config file that are not specified in dvb-v5-std.c::sys_foo_props[] were once loaded to "parms" structure to be scanned, but later discarded in dvb_fe_get_parms() (without experiments/tests :P ). I noticed that there's no effect in this patch by experiments, and non "sys_foo_props[]"-defined props are discarded at its read in the first place. sorry to have bothered you with the meaningless patch. maybe it was because I had a cold recently;) -- Akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 7/7] v4l-utils/libdvbv5: add gconv module for the text conversions of ISDB-S/T.
On 2014年11月01日 04:47, Mauro Carvalho Chehab wrote: > This one failed to build here: > > Making all in gconv > make[3]: Entering directory `/devel/v4l/v4l-utils/lib/gconv' > make[3]: *** No rule to make target `all'. Stop. Sorry, I forgot to "git add -f Makefile". I'll repost v4. > I had to add two extra patches: > > http://git.linuxtv.org/cgit.cgi/v4l-utils.git/commit/?id=22c4ea6ab7fdbc23e68dbfbe5056f90171e9a019 > > http://git.linuxtv.org/cgit.cgi/v4l-utils.git/commit/?id=2e6c685556e589d842bc059b1b1fefb8bc6df865 > > In order to fix Doxygen generation. and thanks for fixing/adding the documentation. I built the doxygen doc and confirmed that countries.h and COUNTRY prop were properly documented. -- Akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git:v4l-utils/master] libdvbv5, dvbv5-scan: generalize channel duplication check
Hi, After I re-checked the source, I noticed that dvb_scan_add_entry() also breaks API/ABI compatibility as well as dvb_new_freq_is_needed(), and those functions are marked as "ancillary functions used internally inside the library" in dvb-scan.h. So I think it would rather be better to move those funcs to a private header (dvb-scan-priv.h?). Which way should we go? ver bump/compat-soname.c/dvb-scan-priv.h ? -- Akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/7] v4l-utils/libdvbv5: add support for ISDB-S scanning
> Hmm... Is this due to a spec definition, or is it just how satellite > operators decided? > > If ARIB spec doesn't allow polarization set, we should remove it from the > Kernel DocBook. ARIB spec does NOT limit polarization, just the satellite operators do. > > Btw, how does the PT1 driver handle those parameters? If it just uses > frequency (it seems so), then the only valid parameters for ISDB-S are > DTV_FREQUENCY > and > DTV_STREAM_ID > > (and, eventually, polarization, if ARIB spec allows it) > > right? Yes, right. PT1 uses DTV_FREQUENCY and DTV_STREAM_ID. -- Akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] v4l-utils/libdvbv5: restore deleted functions to keep API/ABI compatible
Hi, the patch is aimed to do just as you wrote, i.e. add wrapper funcs with the original names. sorry for my bad english. -- akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] v4l-utils/libdvbv5: add gconv module for the text conversions of ISDB-S/T.
Hi, > I would really prefer if you could use the autotools toolchain > (autoconf, automake, libtool) to produce the two gconv modules. You > might be able to have a look at the v4l-plugins Makefiles in this project. As the upstream glibc does not use autotools, I looked through the Makefiles there and they were too complex for me to convert to the simple version for just building out-of-tree modules. So the current Makefile is pretty premitive. But I'll try to investigate it again. > In the existing Makefile I miss an install target. Those modules are not intended to be installed, instead GCONV_PATH is set to the directory at runtime. > Did you write the whole gconv module by yourself? Please clarify > copyright. Because libdvbv5 is useable without the gconv modules I would > move them into /contrib rather than /lib. the work was done by myself but it is based on the other existing modules (iso-2022-jp-3 and iso_6937). I'd like to assign copyrights to FSF as written in a file header, as I intend to contribute them to the upstream glibc. > Are you aware of any other software that ships gconv modules? I'd like > to take a look how it got packaged for distributions. Unfortunately I don't know one, and that's why those gconv modules are so badly packaged;) -- Akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dvb:tc90522: bugfix of always-false expression
I already posted v2 to this ML, but it was at top-level, not in-reply-to this thread. Sorry for the confusing posts. v2 of this patch: https://patchwork.linuxtv.org/patch/26652/ and the other two patches https://patchwork.linuxtv.org/patch/26651/ https://patchwork.linuxtv.org/patch/26653/ regards, Akihiro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media: dvb/earth-pt1: fix wrong initialization for demod blocks
Hi, thank you for reviewing my patch, ( <20190110095623.28070-1-tsk...@gmail.com> https://patchwork.linuxtv.org/patch/53834/ ), and excuse me for my late reply. I somehow lost your mail and noticed it just today by checking Patchwork. On Mon, 18 Feb 2019 21:04:28 +, you wrote: > It might be possible to simplify the code a little by using strcmp() and > making it into one loop, like so: > > for (i = 0; i < PT1_NR_ADAPS; i++) { > cl = pt1->adaps[i]->demod_i2c_client; > if (strcmp(cl->name, TC90522_I2C_DEV_SAT) && > strcmp(cl->name, TC90522_I2C_DEV_TER)) > continue; > > ret = i2c_master_send(cl, buf, 2); > if (ret < 0) > return ret; > > usleep_range(3, 5); > } Any "cl" has the name of either TC90522_I2C_DEV_SAT or TC90522_I2C_DEV_TER, and no other name exists. (adaps[0],adaps[2]: _SAT, adaps[1],adaps[3]: _TER) The purpose of the code is to ensure that TER clients are processed BEFORE SAT clients, as noted in the header comment of this function. So I am afraid that unifying the two loops does not work. -- Akihiro