Message ID | 829464.15115.qm@web110810.mail.gq1.yahoo.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Hi Mauro, Hope you had a great weekend :-) I would like to know if you have already reached the conclusion whether to use the Mercurial tree option or the email option we discussed last week. Regarding the patches that have been already submitted to the linux-media@vger.kernel.org ML, any schedule for the merge? Thanks & Regard, Uri -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Uri Shkolnik wrote: > Hi Mauro, > > Hope you had a great weekend :-) > > > I would like to know if you have already reached the conclusion whether to use the Mercurial tree option or the email option we discussed last week. > > Regarding the patches that have been already submitted to the linux-media@vger.kernel.org ML, any schedule for the merge? > Uri, I've already responded to your last email -- You should continue to submit patches to the mailing list. As for your pending patches, I explained in my email that they are in my queue. I am in the midst of reviewing the changes. As you're already aware, your changes break the Hauppauge device's functionality. After merging your changes, I will have to go back and re-implement the Hauppauge-specific changes in the driver, using the new methods in your pending patches. Once I have reviewed & merged your changes and after I can restore the proper functionality to the hauppauge devices, then I will post a new mercurial tree for testing against all siano-based devices. Please be patient -- this takes time. Regards, Mike -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- On Sun, 1/18/09, Michael Krufky <mkrufky@linuxtv.org> wrote: > From: Michael Krufky <mkrufky@linuxtv.org> > Subject: Re: Siano's patches > To: urishk@yahoo.com > Cc: "Mauro Carvalho" <mchehab@infradead.org>, linux-media@vger.kernel.org, linuxtv-commits@linuxtv.org, "linux-dvb" <linux-dvb@linuxtv.org> > Date: Sunday, January 18, 2009, 8:07 PM > Uri Shkolnik wrote: > > Hi Mauro, > > > > Hope you had a great weekend :-) > > > > > > I would like to know if you have already reached the > conclusion whether to use the Mercurial tree option or the > email option we discussed last week. > > Regarding the patches that have been already submitted > to the linux-media@vger.kernel.org ML, any schedule for the > merge? > Uri, > > I've already responded to your last email -- You should > continue to submit patches to the mailing list. Mauro suggested two options, I asked for the first (the Mercurial tree) which is more convenient for me. As it used by multiple parties here and if there is no objection based on some unknown (to me) reason, I would like to use that option. > > As for your pending patches, I explained in my email that > they are in my queue. I am in the midst of reviewing the > changes. As you're already aware, your changes break > the Hauppauge device's functionality. After merging > your changes, I will have to go back and re-implement the > Hauppauge-specific changes in the driver, using the new > methods in your pending patches. > Some of the patches in the list are pending from early September, 2008. ( I think it's time they will be popped out of the queue.... :-) Regarding restoring, modifying, enhancing, etc. - Please do it successively, submitting my patches and your after them, so (1) the congruity between the Mercurial DVB tree change-sets and Siano's change-set will be kept, and (2) I, and other reviewers, may review your patches/changes in their own change-sets. > Once I have reviewed & merged your changes and after I > can restore the proper functionality to the hauppauge > devices, then I will post a new mercurial tree for testing > against all siano-based devices. > Please see above > Please be patient -- this takes time. > > Regards, > > Mike > -- > To unsubscribe from this list: send the line > "unsubscribe linux-media" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at > http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Uri, On Sun, 18 Jan 2009 10:37:32 -0800 (PST) Uri Shkolnik <urishk@yahoo.com> wrote: > > From: Michael Krufky <mkrufky@linuxtv.org> > > Subject: Re: Siano's patches > > To: urishk@yahoo.com > > Cc: "Mauro Carvalho" <mchehab@infradead.org>, linux-media@vger.kernel.org, linuxtv-commits@linuxtv.org, "linux-dvb" <linux-dvb@linuxtv.org> > > Date: Sunday, January 18, 2009, 8:07 PM > > Uri Shkolnik wrote: > > > Hi Mauro, > > > > > > Hope you had a great weekend :-) Yes, I had ;) > > > > > > > > > I would like to know if you have already reached the > > conclusion whether to use the Mercurial tree option or the > > email option we discussed last week. > > > Regarding the patches that have been already submitted > > to the linux-media@vger.kernel.org ML, any schedule for the > > merge? > > Uri, > > > > I've already responded to your last email -- You should > > continue to submit patches to the mailing list. > > Mauro suggested two options, I asked for the first (the Mercurial tree) which is more convenient for me. As it used by multiple parties here and if there is no objection based on some unknown (to me) reason, I would like to use that option. > > > > > > As for your pending patches, I explained in my email that > > they are in my queue. I am in the midst of reviewing the > > changes. As you're already aware, your changes break > > the Hauppauge device's functionality. After merging > > your changes, I will have to go back and re-implement the > > Hauppauge-specific changes in the driver, using the new > > methods in your pending patches. > > > > Some of the patches in the list are pending from early September, 2008. > ( I think it's time they will be popped out of the queue.... :-) > > Regarding restoring, modifying, enhancing, etc. - Please do it successively, submitting my patches and your after them, so (1) the congruity between the Mercurial DVB tree change-sets and Siano's change-set will be kept, and (2) I, and other reviewers, may review your patches/changes in their own change-sets. > > > > Once I have reviewed & merged your changes and after I > > can restore the proper functionality to the hauppauge > > devices, then I will post a new mercurial tree for testing > > against all siano-based devices. > > > > Please see above > > > Please be patient -- this takes time. Since the current Siano implementation is needed by some existing cards and that, from what I understood from your request and Michael comments, those patches could cause some regressions with the supported boards, we should wait for Michael's reviews and tests, if this doesn't take that long. Uri, I'm assuming that you're familiar with kernel development. If not, I recommend you to read README.patches and the related docs at kernel's Documentation tree. If you have any doubts about that, we're here to answer. Those patches are late for kernel 2.6.29, since the merge window for that kernel were already closed. So, we will have some time until the next open window opens (we should have at least 5 weeks). This allows us to do a better review the changesets to be sure that: they don't cause any regressions; they don't violate the DVB API; they don't create undocumented userspace API's; they are using the best development practices on kernel development; individual patches don't break compilation (to avoid breaking git bissect). For a large changeset like Siano ones, such reviews take time. Ah, next time, please number your changesets with something like [PATCH 01/xx]. This helps me to properly identify and honour the correct patch order. Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- On Mon, 1/19/09, Mauro Carvalho Chehab <mchehab@infradead.org> wrote: > From: Mauro Carvalho Chehab <mchehab@infradead.org> > Subject: Re: Siano's patches > To: urishk@yahoo.com > Cc: "Michael Krufky" <mkrufky@linuxtv.org>, linux-media@vger.kernel.org, linuxtv-commits@linuxtv.org, "linux-dvb" <linux-dvb@linuxtv.org> > Date: Monday, January 19, 2009, 1:29 PM > Hi Uri, > > On Sun, 18 Jan 2009 10:37:32 -0800 (PST) > Uri Shkolnik <urishk@yahoo.com> wrote: > > > > From: Michael Krufky <mkrufky@linuxtv.org> > > > Subject: Re: Siano's patches > > > To: urishk@yahoo.com > > > Cc: "Mauro Carvalho" > <mchehab@infradead.org>, linux-media@vger.kernel.org, > linuxtv-commits@linuxtv.org, "linux-dvb" > <linux-dvb@linuxtv.org> > > > Date: Sunday, January 18, 2009, 8:07 PM > > > Uri Shkolnik wrote: > > > > Hi Mauro, > > > > > > > > Hope you had a great weekend :-) > > Yes, I had ;) > > > > > > > > > > > > > I would like to know if you have already > reached the > > > conclusion whether to use the Mercurial tree > option or the > > > email option we discussed last week. > > > > Regarding the patches that have been already > submitted > > > to the linux-media@vger.kernel.org ML, any > schedule for the > > > merge? > > > Uri, > > > > > > I've already responded to your last email -- > You should > > > continue to submit patches to the mailing list. > > > > Mauro suggested two options, I asked for the first > (the Mercurial tree) which is more convenient for me. As it > used by multiple parties here and if there is no objection > based on some unknown (to me) reason, I would like to use > that option. > > > > > > > > > > As for your pending patches, I explained in my > email that > > > they are in my queue. I am in the midst of > reviewing the > > > changes. As you're already aware, your > changes break > > > the Hauppauge device's functionality. After > merging > > > your changes, I will have to go back and > re-implement the > > > Hauppauge-specific changes in the driver, using > the new > > > methods in your pending patches. > > > > > > > Some of the patches in the list are pending from early > September, 2008. > > ( I think it's time they will be popped out of the > queue.... :-) > > > > Regarding restoring, modifying, enhancing, etc. - > Please do it successively, submitting my patches and your > after them, so (1) the congruity between the Mercurial DVB > tree change-sets and Siano's change-set will be kept, > and (2) I, and other reviewers, may review your > patches/changes in their own change-sets. > > > > > > > Once I have reviewed & merged your changes > and after I > > > can restore the proper functionality to the > hauppauge > > > devices, then I will post a new mercurial tree > for testing > > > against all siano-based devices. > > > > > > > Please see above > > > > > Please be patient -- this takes time. > > Since the current Siano implementation is needed by some > existing cards and > that, from what I understood from your request and Michael > comments, those > patches could cause some regressions with the supported > boards, we should wait > for Michael's reviews and tests, if this doesn't > take that long. > > Uri, I'm assuming that you're familiar with kernel > development. If not, I > recommend you to read README.patches and the related docs > at kernel's > Documentation tree. If you have any doubts about that, > we're here to answer. > > Those patches are late for kernel 2.6.29, since the merge > window for that > kernel were already closed. So, we will have some time > until the next open > window opens (we should have at least 5 weeks). This allows > us to do a better > review the changesets to be sure that: > they don't cause any regressions; > they don't violate the DVB API; > they don't create undocumented userspace API's; > they are using the best development practices on kernel > development; > individual patches don't break compilation (to avoid > breaking git bissect). > > For a large changeset like Siano ones, such reviews take > time. > > Ah, next time, please number your changesets with something > like [PATCH 01/xx]. > This helps me to properly identify and honour the correct > patch order. > > Cheers, > Mauro Hi Mauro, Thanks for your detailed response. Some comments and remarks - Please note that the vast majority of the added (new files) code is SDIO and SPI bus interfaces, (and also big endian support). I don't know anyone reading this ML (Mike is included), who has the tools to test this code, which has been tested thoroughly. However, comments about coding style and kernel-coding related remarks are most welcome (I already submitted a patch for review to fix some endianity issue Trent Piepho commented about). Please note that the SDIO patches has been written by none other than Pierre Ossman, who is the Linux kernel MMC maintainer (who wrote this code back in August 2008). Siano has some dozens of commercial Linux-based customers using the discussed sources. Those customers have their own QA engineers additionally to Siano internal QA team (which includes dedicated engineer for this task). Some of those companies products are already in the market (production level). Second note, regarding regressions - the current code in the Mercurial tree includes code that Mike added without any review, which manipulate the chip-set's general purpose I/O. This code includes logical errors that causes boards with different layouts than Mike's to either deteriorate in performances (reception quality) or to stop functioning altogether. This issue got "blocker" status on my issues' list (the most severe type on my scale) and it prevents the various companies and individuals to use the Mercurial (and the kernel git) as a reliable source for Linux TV. I am fully aware that Mike need to add/modify some code in order to support current/additional HPG cards (boards). All those modification are (1) Related to single source file (sms-cards.c) and (2) Can and should be done *after* submitting Siano's patches. (3) Pass a review, like any other patch, *before* submitting them to the main Mercurial tree. Regarding "violate the DVB API" & "don't create undocumented userspace API's" - I guess you refers to the 'Siano Sub-system'. Well, .... this subject has been discussed a lot, and I don't mind at all to give it another round of discussions... DVB v3 never supported CMMB, T-DMB, ISDB-T and some other MDTV standards. DVB v5/S2 yet to support them. Those standards are used by ~2 Billion people. (yep, only little countries like Japan, China, S. Korea, Brazil, the entire Middle East and the Arab peninsula and "very few" others countries use these standards).... :-)))) Siano added some kernel support (tiny sub system) and user-space library. The kernel code is of course GPLv2. *Everything* is available to *all*, include detailed documentation. You can see on the ML some recent posts about it (people who use it to get DAB streams with very little effort). I strongly support any effort (and will help it myself) to have a single, unified, all-in-one API that will support all DTV standards. But we are not there, we are not even close :-((( Until that time, when such generic, all-inclusive API will be introduced, we still have to support those "negligible" ~2 Billion people with some kind of additional API, *only* for those unsupported DTV standards. Best regard, Uri -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
(Oops, looks like I'm losing references, sorry) Uri wrote... >Siano added some kernel support (tiny sub system) and user-space library. >The kernel code is of course GPLv2. >*Everything* is available to *all*, include detailed documentation. You can see > on the ML some recent posts about it (people who use it to get DAB streams wit >h very little effort). Of course, I'm assuming this user-space library is a substitute for a full-fledged API, and exists to give end-users the ability to at least make more use of their hardware than is presently possible with the lack-of-API. I'm happy to be able to tune DAB, though the library functionality is limited and essentially it's like a computer-interface to an ordinary desktop DAB radio, with the benefit of getting the actual payload, or something resembling it (the original DAB error- detection and -correction leaves much to be desired). That would be fine for the majority of users, I expect, but as you noted, with this library it's not possible to get at the raw ensemble data for whatever reason, study of the data within, and the like. So it seems this is somewhat like the `dabusb' source that already exists for one specific receiver, neither of which are a good basis for starting on an API, but which at least bring something to the end-user who today, due to lack of a common API to get access to DAB/DAB+ and the many other widespread standards that Uri mentioned, but which, sadly, so far few companies are producing hardware for, that can be used in a generic Linux box, what a run-on sentence this is, and I've forgotten my point already. barry bouwsma speaking of regressions, either someone broke the 44BSD UFS filesystem in the latest kernel I'm working with, or I've hosed more superblocks in my attempt to upgrade. And I'd rather be spending my time trying to figure out why I'm not getting the clean DAB reception I expect, having eliminated some faulty hardware I haven't been able to test until now... -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 19 Jan 2009, Uri Shkolnik wrote: > Siano has some dozens of commercial Linux-based customers using the > discussed sources. Those customers have their own QA engineers > additionally to Siano internal QA team (which includes dedicated engineer > for this task). Some of those companies products are already in the > market (production level). But how much testing do you give other manufacturers' hardware with your code? Your hardware might work, but you could have broken someone else's. I've found that getting patches into the kernel is usually significantly harder than writing them in the first place. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- On Tue, 1/20/09, Trent Piepho <xyzzy@speakeasy.org> wrote: > From: Trent Piepho <xyzzy@speakeasy.org> > Subject: Re: Siano's patches > To: "Uri Shkolnik" <urishk@yahoo.com> > Cc: "Mauro Carvalho Chehab" <mchehab@infradead.org>, "Michael Krufky" <mkrufky@linuxtv.org>, linux-media@vger.kernel.org, "linux-dvb" <linux-dvb@linuxtv.org> > Date: Tuesday, January 20, 2009, 3:49 AM > On Mon, 19 Jan 2009, Uri Shkolnik wrote: > > Siano has some dozens of commercial Linux-based > customers using the > > discussed sources. Those customers have their own QA > engineers > > additionally to Siano internal QA team (which includes > dedicated engineer > > for this task). Some of those companies products are > already in the > > market (production level). > > But how much testing do you give other manufacturers' > hardware with your > code? Your hardware might work, but you could have broken > someone else's. > > I've found that getting patches into the kernel is > usually significantly > harder than writing them in the first place. mmm.... You have a good point here. I don't know, there is a kind of catch-22 here. True the code may break someone else', or violate something unknown to me. But, how can it be tested if you don't have suitable hardware? Take the SPI as example. The code is for system with PXA CPU, which is connected with SPI/SPP bus to the SMS DTV chip-set. You need such (hw) device in order to check/test it. Who has this hardware but some manufacturers? Maybe someone bought & hacked such a commercial device, but I never got any indication that anyone did such a thing. You can of course just compile the code and see if the build is completed successfully, without running it on a real target. This is some kind of code testing, and that is what I referred to as "kernel-coding related remarks" in my post. Uri -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Michael, On Sun, 18 Jan 2009 13:07:34 -0500 Michael Krufky <mkrufky@linuxtv.org> wrote: > Once I have reviewed & merged your changes and after I can restore the > proper functionality to the hauppauge devices, then I will post a new > mercurial tree for testing against all siano-based devices. > > Please be patient -- this takes time. Any news? Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Mauro Carvalho Chehab wrote: > Michael, > > On Sun, 18 Jan 2009 13:07:34 -0500 > Michael Krufky <mkrufky@linuxtv.org> wrote: > > > >> Once I have reviewed & merged your changes and after I can restore the >> proper functionality to the hauppauge devices, then I will post a new >> mercurial tree for testing against all siano-based devices. >> >> Please be patient -- this takes time. >> > > Any news? I haven't had time to fix the breakage that these changes caused on the Hauppauge devices. This is what I will do -- I will gather all the patches up and push them all into one tree, regardless of any codingstyle issues or breakages, just so that we at least have track of everything. Once it's all in one repository, I can just move the gpio function that the hauppauge devices depend on into the sms-cards.c file -- this way, they will not be used by other devices and the siano code can go in as desired by Uri. The first tree that gets pushed up will likely need some cleanup, but we'll deal with that afterwards. I saw some other interesting patches that Uri's been posting on the lists that I'd definitely like to merge along with these as soon as possible. -Mike -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff -r 50775d37b854 -r 56b506432a55 linux/drivers/media/dvb/siano/smscoreapi.c --- a/linux/drivers/media/dvb/siano/smscoreapi.c Thu Jan 15 15:39:15 2009 +0200 +++ b/linux/drivers/media/dvb/siano/smscoreapi.c Sun Jan 18 11:32:50 2009 +0200 @@ -481,8 +481,8 @@ static int smscore_load_firmware_family2 u8 *payload = firmware->Payload; int rc = 0; - firmware->StartAddress = __le32_to_cpu(firmware->StartAddress); - firmware->Length = __le32_to_cpu(firmware->Length); + firmware->StartAddress = le32_to_cpu(firmware->StartAddress); + firmware->Length = le32_to_cpu(firmware->Length); mem_address = firmware->StartAddress; diff -r 50775d37b854 -r 56b506432a55 linux/drivers/media/dvb/siano/smsendian.c --- a/linux/drivers/media/dvb/siano/smsendian.c Thu Jan 15 15:39:15 2009 +0200 +++ b/linux/drivers/media/dvb/siano/smsendian.c Sun Jan 18 11:32:50 2009 +0200 @@ -34,7 +34,7 @@ void smsendian_handle_tx_message(void *b switch (msg->xMsgHeader.msgType) { case MSG_SMS_DATA_DOWNLOAD_REQ: { - msg->msgData[0] = __le32_to_cpu(msg->msgData[0]); + msg->msgData[0] = le32_to_cpu(msg->msgData[0]); break; } @@ -43,7 +43,7 @@ void smsendian_handle_tx_message(void *b sizeof(struct SmsMsgHdr_ST))/4; for (i = 0; i < msgWords; i++) - msg->msgData[i] = __le32_to_cpu(msg->msgData[i]); + msg->msgData[i] = le32_to_cpu(msg->msgData[i]); break; } @@ -62,7 +62,7 @@ void smsendian_handle_rx_message(void *b { struct SmsVersionRes_ST *ver = (struct SmsVersionRes_ST *) msg; - ver->ChipModel = __le16_to_cpu(ver->ChipModel); + ver->ChipModel = le16_to_cpu(ver->ChipModel); break; } @@ -79,7 +79,7 @@ void smsendian_handle_rx_message(void *b sizeof(struct SmsMsgHdr_ST))/4; for (i = 0; i < msgWords; i++) - msg->msgData[i] = __le32_to_cpu(msg->msgData[i]); + msg->msgData[i] = le32_to_cpu(msg->msgData[i]); break; } @@ -92,9 +92,9 @@ void smsendian_handle_message_header(voi #ifdef __BIG_ENDIAN struct SmsMsgHdr_ST *phdr = (struct SmsMsgHdr_ST *)msg; - phdr->msgType = __le16_to_cpu(phdr->msgType); - phdr->msgLength = __le16_to_cpu(phdr->msgLength); - phdr->msgFlags = __le16_to_cpu(phdr->msgFlags); + phdr->msgType = le16_to_cpu(phdr->msgType); + phdr->msgLength = le16_to_cpu(phdr->msgLength); + phdr->msgFlags = le16_to_cpu(phdr->msgFlags); #endif /* __BIG_ENDIAN */ } diff -r 50775d37b854 -r 56b506432a55 linux/drivers/media/dvb/siano/smsusb.c --- a/linux/drivers/media/dvb/siano/smsusb.c Thu Jan 15 15:39:15 2009 +0200 +++ b/linux/drivers/media/dvb/siano/smsusb.c Sun Jan 18 11:32:50 2009 +0200 @@ -334,7 +334,7 @@ static int smsusb_init_device(struct usb case SMS_VEGA: dev->buffer_size = USB2_BUFFER_SIZE; dev->response_alignment = - __le16_to_cpu(dev->udev->ep_in[1]->desc.wMaxPacketSize) - + le16_to_cpu(dev->udev->ep_in[1]->desc.wMaxPacketSize) - sizeof(struct SmsMsgHdr_ST); params.flags |= SMS_DEVICE_FAMILY2;