Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1345228 > unrolled thread

Re: [PATCH v2 1/4] USB: mxu11x0: fix memory leak on usb_serial private data

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2 1/4] USB: mxu11x0: fix memory leak on usb_serial  private data Johan Hovold <johan@kernel.org> - 2016-02-28 13:30 +0100
    Re: [PATCH v2 1/4] USB: mxu11x0: fix memory leak on usb_serial  private data Mathieu OTHACEHE <m.othacehe@gmail.com> - 2016-02-28 17:40 +0100
      Re: [PATCH v2 1/4] USB: mxu11x0: fix memory leak on usb_serial  private data Johan Hovold <johan@kernel.org> - 2016-02-29 10:40 +0100

#1345228 — Re: [PATCH v2 1/4] USB: mxu11x0: fix memory leak on usb_serial private data

On Sat, Jan 30, 2016 at 06:40:30PM +0100, Mathieu OTHACEHE wrote:
> On Mon, Jan 25, 2016 at 01:01:59PM +0100, Johan Hovold wrote:
> > On Mon, Jan 04, 2016 at 07:49:36PM +0100, Mathieu OTHACEHE wrote:
> > > On nominal execution, private data allocated on port_probe and attach
> > > are never freed. Add port_remove and release callbacks to free them
> > > respectively.
> > > 
> > > Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
> > 
> > I've applied this one for 4.5-rc2 now.
> > 
> > I want to take a closer look at the last three patches and it seems they
> > should wait for 4.6 anyway. I did notice that the vendor driver also
> > sends double START/OPEN commands at open by the way. Perhaps ask Moxa
> > why that is before we remove them?
> > 
> > Thanks,
> > Johan
> 
> Hi Johan,
> 
> I asked MOXA about this double opening. I also noticed that the
> mainline driver ti_usb_3410_5052 uses the same double opening pattern.
> And, MOXA UPORT 11x0 serie is based on TUSB3410 chip of TI.
> 
> So, I also emailed TI, and the authors of ti_usb_3410_5052 driver.

Wow, this is embarrassing. I only now noticed that the mxu11x0 driver,
well at least prior to all your clean-ups, is almost identical to the
ti_usb_3410_5052 driver, and here I see you mention that it is indeed
based on the same chip.

I wish that this had been made clear from the outset. We don't want two
drivers for the same chip if we can avoid it. Instead we should try to
merge these changes back to the ti_usb_3410_5052 driver and clean that
up instead.

Do you see anything preventing us from using the ti_usb_3410_5052
driver for these Moxa devices?

Thanks,
Johan

[toc] | [next] | [standalone]

#1345349

On Sun, Feb 28, 2016 at 01:20:16PM +0100, Johan Hovold wrote:
> On Sat, Jan 30, 2016 at 06:40:30PM +0100, Mathieu OTHACEHE wrote:
> > On Mon, Jan 25, 2016 at 01:01:59PM +0100, Johan Hovold wrote:
> > > On Mon, Jan 04, 2016 at 07:49:36PM +0100, Mathieu OTHACEHE wrote:
> > > > On nominal execution, private data allocated on port_probe and attach
> > > > are never freed. Add port_remove and release callbacks to free them
> > > > respectively.
> > > > 
> > > > Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
> > > 
> > > I've applied this one for 4.5-rc2 now.
> > > 
> > > I want to take a closer look at the last three patches and it seems they
> > > should wait for 4.6 anyway. I did notice that the vendor driver also
> > > sends double START/OPEN commands at open by the way. Perhaps ask Moxa
> > > why that is before we remove them?
> > > 
> > > Thanks,
> > > Johan
> > 
> > Hi Johan,
> > 
> > I asked MOXA about this double opening. I also noticed that the
> > mainline driver ti_usb_3410_5052 uses the same double opening pattern.
> > And, MOXA UPORT 11x0 serie is based on TUSB3410 chip of TI.
> > 
> > So, I also emailed TI, and the authors of ti_usb_3410_5052 driver.
> 
> Wow, this is embarrassing. I only now noticed that the mxu11x0 driver,
> well at least prior to all your clean-ups, is almost identical to the
> ti_usb_3410_5052 driver, and here I see you mention that it is indeed
> based on the same chip.
> 
> I wish that this had been made clear from the outset. We don't want two
> drivers for the same chip if we can avoid it. Instead we should try to
> merge these changes back to the ti_usb_3410_5052 driver and clean that
> up instead.
> 
> Do you see anything preventing us from using the ti_usb_3410_5052
> driver for these Moxa devices?

Hi Johan,

No, I don't see any problem to do that. I am testing ti_usb_3410_5052
with support for MOXA 11x0 and almost everything seems fine.
Sorry I didn't noticed it before, it would have saved us some time.

So, I could post a patch serie :

1. Removing mxu11x0 driver
2. Patching ti_usb_3410_5052
3. Cleaning up ti_usb_3410_5052 the same as we cleaned-up mxu11x0

Btw, no response of TI or MOXA about the double opening stuff.

Thank you,
Mathieu

[toc] | [prev] | [next] | [standalone]

#1345600

On Sun, Feb 28, 2016 at 05:30:54PM +0100, Mathieu OTHACEHE wrote:
> On Sun, Feb 28, 2016 at 01:20:16PM +0100, Johan Hovold wrote:

> So, I could post a patch serie :
> 
> 1. Removing mxu11x0 driver
> 2. Patching ti_usb_3410_5052
> 3. Cleaning up ti_usb_3410_5052 the same as we cleaned-up mxu11x0

Please do. If we could bring ti_usb_3410_5052 to the same state as
mxu11x0 is today that's be great.

> Btw, no response of TI or MOXA about the double opening stuff.

Never mind that for now. Moxa apparently just copied the TI driver and
ran search and replace on the symbol prefixes. I assume they won't have
an answer.

Thanks,
Johan

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web