Re: 转发: [PATCH] usb: serial: update CH34x driver in drivers/usb/serial

Re: 转发: [PATCH] usb: serial: update CH34x driver in drivers/usb/serial

[Greg KH]

On Fri, Jun 24, 2016 at 01:42:24PM +0800, WCH Tech Group wrote:
> 	There are several reasons why we decided to revoke the old one after
> communicating with the author of 
> ch341.c (Frank A Kingswood <frank@kingswood-consulting.co.uk>), first of all
> we want the driver to support both ch341 and
> ch340 chips, so we changed the driver name from "ch341.c" to "ch34x.c",

No need to rename the driver to support multiple chips.  Keep it the
same name, and just add the new device support.  That's how we do it for
lots and lots of Linux drivers, the name doesn't really matter that
much (look at the option.c driver for one such example.)

> secondly the new driver and old one are coded
> by different authors, in fact there's no connection between them.

Ok, but the functionality is the same, so please just fix up the
existing driver to add support for the new device, and fix any existing
bugs.

In Linux you don't get to just delete a working driver, you have to
evolve code over time, sending patches that do one logical thing at a
time so that people can properly review them.  Your patch is not how
this is supposed to happen at all.

So please just break up your changes into small logical ones, and send a
series of patches adding the new device support and fix up any known
bugs.

After that is all done, if you _really_ want to rename the driver, then
we can discuss that, but first do the work to evolve the driver, as that
is much more difficult.

thanks,

greg k-h

Re: 转发: [PATCH] usb: serial: update CH34x driver in drivers/usb/serial

[Greg KH]

On Thu, Sep 15, 2016 at 12:03:48AM +0100, Aidan Thornton wrote:
> On 24 Jun 2016 16:10, "Greg KH" <gregkh@linuxfoundation.org> wrote:
> >
> > On Fri, Jun 24, 2016 at 01:42:24PM +0800, WCH Tech Group wrote:
> > >       There are several reasons why we decided to revoke the old one after
> > > communicating with the author of
> > > ch341.c (Frank A Kingswood <frank@kingswood-consulting.co.uk>), first of
> all
> > > we want the driver to support both ch341 and
> > > ch340 chips, so we changed the driver name from "ch341.c" to "ch34x.c",
> >
> > No need to rename the driver to support multiple chips.  Keep it the
> > same name, and just add the new device support.  That's how we do it for
> > lots and lots of Linux drivers, the name doesn't really matter that
> > much (look at the option.c driver for one such example.)
> >
> > > secondly the new driver and old one are coded
> > > by different authors, in fact there's no connection between them.
> >
> > Ok, but the functionality is the same, so please just fix up the
> > existing driver to add support for the new device, and fix any existing
> > bugs.
> >
> > In Linux you don't get to just delete a working driver, you have to
> > evolve code over time, sending patches that do one logical thing at a
> > time so that people can properly review them.  Your patch is not how
> > this is supposed to happen at all.
> >
> > So please just break up your changes into small logical ones, and send a
> > series of patches adding the new device support and fix up any known
> > bugs.
> >
> > After that is all done, if you _really_ want to rename the driver, then
> > we can discuss that, but first do the work to evolve the driver, as that
> > is much more difficult.
> >
> > thanks,
> >
> > greg k-h
> 
> It looks like someone by the name of Grigori Goronzy (CCed) had a patch series
> or four attempting to do this that just never went anywhere like all the other
> attempts. Might be worth someone talking to him or looking at his patches.

Do you have a pointer to those patches on the mailing list?  Why were
they rejected?

> Seriously, this is... I was considering trying to get parity support merged so
> I don't have to keep patching it in, but it feels like a total waste of effort
> at this point after seeing all the other attempts.

No reason you can't take those patches and fix them up and resend them,
right?

thanks,

greg k-h

Re: 转发: [PATCH] usb: serial: update CH34x driver in drivers/usb/serial

[Aidan Thornton]

On Thu, Sep 15, 2016 at 6:56 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Thu, Sep 15, 2016 at 12:03:48AM +0100, Aidan Thornton wrote:
>>
>> It looks like someone by the name of Grigori Goronzy (CCed) had a patch series
>> or four attempting to do this that just never went anywhere like all the other
>> attempts. Might be worth someone talking to him or looking at his patches.
>
> Do you have a pointer to those patches on the mailing list?  Why were
> they rejected?

https://lkml.org/lkml/2016/4/15/849
Not entirely sure. I think mainly some quibble with an unrelated
resume issue in the first patch and some objection to RTS/CTS and B0
handling for it being in separate patches, but but there's 13 patches
in the series, four revisions of it, and probably other issues waiting
to be discovered by the next fool who tries to resubmit them.

>
>> Seriously, this is... I was considering trying to get parity support merged so
>> I don't have to keep patching it in, but it feels like a total waste of effort
>> at this point after seeing all the other attempts.
>
> No reason you can't take those patches and fix them up and resend them,
> right?

Anything more complicated than parity support apparently runs into
hardware quirks which unlike the author I simply don't have the
hardware to test. Everything I've got uses the CH340G and all the
people who do have the right hardware appear to have quite
understandably given up. Would require buying more hardware from
China, waiting for it to ship, spending a few hours with a logic
analyzer, praying that this hasn't broken some other earlier chip
revision, and likely all for nothing.

Might consider it anyway but it makes more sense to keep on pointing
people to the out-of-tree parity patch just like everyone else has
been doing for the past two and a half years. Especially since most
users will still need it for quite some time even if support does get
merged.

Re: 转发: [PATCH] usb: serial: update CH34x driver in drivers/usb/serial

[Johan Hovold]

On Thu, Sep 15, 2016 at 11:19:26AM +0100, Aidan Thornton wrote:
> On Thu, Sep 15, 2016 at 6:56 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Thu, Sep 15, 2016 at 12:03:48AM +0100, Aidan Thornton wrote:
> >>
> >> It looks like someone by the name of Grigori Goronzy (CCed) had a patch series
> >> or four attempting to do this that just never went anywhere like all the other
> >> attempts. Might be worth someone talking to him or looking at his patches.
> >
> > Do you have a pointer to those patches on the mailing list?  Why were
> > they rejected?
> 
> https://lkml.org/lkml/2016/4/15/849
> Not entirely sure. I think mainly some quibble with an unrelated
> resume issue in the first patch and some objection to RTS/CTS and B0
> handling for it being in separate patches, but but there's 13 patches
> in the series, four revisions of it, and probably other issues waiting
> to be discovered by the next fool who tries to resubmit them.

The patches were going through the normal review process. Anyone with
access to hardware should be able to complete that series in a few
hours. The four revisions had more to do with new versions being posted
quickly (or initially to the serial list) IIRC.

Johan

Re: 转发: [PATCH] usb: serial: update CH34x driver in drivers/usb/serial

[Grigori Goronzy]

On 2016-09-15 01:03, Aidan Thornton wrote:
>> 
>> thanks,
>> 
>> greg k-h
> 
> It looks like someone by the name of Grigori Goronzy (CCed) had a
> patch series or four attempting to do this that just never went
> anywhere like all the other attempts. Might be worth someone talking
> to him or looking at his patches. Seriously, this is... I was
> considering trying to get parity support merged so I don't have to
> keep patching it in, but it feels like a total waste of effort at this
> point after seeing all the other attempts.

Hi everyone,

sorry for not following up on the patches.  There were some odd 
compatibility issues reported by one user and then I got busy with other 
things, so this got sidetracked.  But just a few days ago the lack of 
working parity annoyed me again. STM32 bootloaders need parity.  I'll 
take a look at your revised patches, Aidan.  Maybe we can finally get 
this done.

About the compatibility issues, I never managed to reproduce them and I 
concluded in the end that it might be something unrelated.

The person that reported compatibility issues with the patch series was 
Eddi De Pieri <eddi@depieri.net>.

Best regards
Grigori