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

Groups > linux.kernel > #1220250 > unrolled thread

similar files: fusbh200-hcd.c and fotg210-hcd.c

Back to article view | Back to linux.kernel

Contents

  similar files: fusbh200-hcd.c and fotg210-hcd.c Peter Senna Tschudin <peter.senna@gmail.com> - 2015-09-07 16:50 +0200
    Re: similar files: fusbh200-hcd.c and fotg210-hcd.c Felipe Balbi <balbi@ti.com> - 2015-09-08 18:00 +0200
      Re: similar files: fusbh200-hcd.c and fotg210-hcd.c Peter Senna Tschudin <peter.senna@gmail.com> - 2015-09-12 15:20 +0200
        Re: similar files: fusbh200-hcd.c and fotg210-hcd.c Felipe Balbi <balbi@ti.com> - 2015-09-14 17:10 +0200
          Re: similar files: fusbh200-hcd.c and fotg210-hcd.c Peter Senna Tschudin <peter.senna@gmail.com> - 2015-09-14 20:00 +0200
            Re: similar files: fusbh200-hcd.c and fotg210-hcd.c Felipe Balbi <balbi@ti.com> - 2015-09-15 16:40 +0200
              Re: similar files: fusbh200-hcd.c and fotg210-hcd.c Peter Senna Tschudin <peter.senna@gmail.com> - 2015-09-15 18:50 +0200
                Re: similar files: fusbh200-hcd.c and fotg210-hcd.c Felipe Balbi <balbi@ti.com> - 2015-09-15 19:00 +0200
                  [PATCH 08/14] RFC: usb/host/fotg210: convert macro to inline function Peter Senna Tschudin <peter.senna@gmail.com> - 2015-09-21 17:10 +0200
                    Re: [PATCH 08/14] RFC: usb/host/fotg210: convert macro to inline  function Felipe Balbi <balbi@ti.com> - 2015-10-02 19:40 +0200
                  [PATCH 09/14] RFC: usb/host/fotg210: Add function: output_buf_tds_dir() Peter Senna Tschudin <peter.senna@gmail.com> - 2015-09-21 17:10 +0200
                    Re: [PATCH 09/14] RFC: usb/host/fotg210: Add function:  output_buf_tds_dir() Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2015-09-21 21:20 +0200
                    Re: [PATCH 09/14] RFC: usb/host/fotg210: Add function:  output_buf_tds_dir() Felipe Balbi <balbi@ti.com> - 2015-10-02 19:40 +0200
                  [PATCH 14/14] RFC: usb/host/faraday-hcd: Import FUSBH200 parameters Peter Senna Tschudin <peter.senna@gmail.com> - 2015-09-21 17:10 +0200
                    Re: [PATCH 14/14] RFC: usb/host/faraday-hcd: Import FUSBH200  parameters Felipe Balbi <balbi@ti.com> - 2015-10-02 19:40 +0200
                  [PATCH 00/14] RFC: Consolidation: FUSB200 and FOTG210 Peter Senna Tschudin <peter.senna@gmail.com> - 2015-09-21 17:10 +0200
                    [PATCH 04/14] RFC: usb/host/fotg210: Remove NULL checks dma_pool_destroy Peter Senna Tschudin <peter.senna@gmail.com> - 2015-09-21 17:10 +0200
                      Re: [PATCH 04/14] RFC: usb/host/fotg210: Remove NULL checks  dma_pool_destroy Felipe Balbi <balbi@ti.com> - 2015-10-02 19:40 +0200
                    [PATCH 03/14] RFC: usb/host/fotg210: Remove useless else statement Peter Senna Tschudin <peter.senna@gmail.com> - 2015-09-21 17:10 +0200
                      Re: [PATCH 03/14] RFC: usb/host/fotg210: Remove useless else  statement Felipe Balbi <balbi@ti.com> - 2015-10-02 19:40 +0200
                    [PATCH 10/14] RFC: usb/host/fotg210: Add function scan_frame_queue() Peter Senna Tschudin <peter.senna@gmail.com> - 2015-09-21 17:10 +0200
                      Re: [PATCH 10/14] RFC: usb/host/fotg210: Add function  scan_frame_queue() Felipe Balbi <balbi@ti.com> - 2015-10-02 19:40 +0200
                    [PATCH 06/14] RFC: usb/host/fotg210: replace msleep by usleep_range Peter Senna Tschudin <peter.senna@gmail.com> - 2015-09-21 17:10 +0200
                      Re: [PATCH 06/14] RFC: usb/host/fotg210: replace msleep by  usleep_range Felipe Balbi <balbi@ti.com> - 2015-10-02 19:40 +0200
                        Re: [PATCH 06/14] RFC: usb/host/fotg210: replace msleep by usleep_range Alan Stern <stern@rowland.harvard.edu> - 2015-10-02 20:00 +0200
                          Re: [PATCH 06/14] RFC: usb/host/fotg210: replace msleep by usleep_range Peter Senna Tschudin <peter.senna@gmail.com> - 2015-10-04 12:00 +0200
                            Re: [PATCH 06/14] RFC: usb/host/fotg210: replace msleep by usleep_range Alan Stern <stern@rowland.harvard.edu> - 2015-10-04 17:30 +0200
                    [PATCH 02/14] RFC: usb/host/fotg210: remove KERN_WARNING from pr_info Peter Senna Tschudin <peter.senna@gmail.com> - 2015-09-21 17:10 +0200
                      Re: [PATCH 02/14] RFC: usb/host/fotg210: remove KERN_WARNING from  pr_info Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2015-09-21 21:00 +0200
                      Re: [PATCH 02/14] RFC: usb/host/fotg210: remove KERN_WARNING from  pr_info Felipe Balbi <balbi@ti.com> - 2015-10-02 19:40 +0200
                    [PATCH 05/14] RFC: usb/host/fotg210: change kmalloc by kmalloc_array Peter Senna Tschudin <peter.senna@gmail.com> - 2015-09-21 17:10 +0200
                      Re: [PATCH 05/14] RFC: usb/host/fotg210: change kmalloc by  kmalloc_array Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2015-09-21 21:00 +0200
                      Re: [PATCH 05/14] RFC: usb/host/fotg210: change kmalloc by  kmalloc_array Felipe Balbi <balbi@ti.com> - 2015-10-02 19:40 +0200
                    [PATCH 13/14] RFC: usb/host/faraday-hcd: Move #defines outside struct Peter Senna Tschudin <peter.senna@gmail.com> - 2015-09-21 17:10 +0200
                      Re: [PATCH 13/14] RFC: usb/host/faraday-hcd: Move #defines outside  struct Felipe Balbi <balbi@ti.com> - 2015-10-02 19:40 +0200
                    [PATCH 07/14] RFC: usb/host/fotg210: Remove a macro from snprintf Peter Senna Tschudin <peter.senna@gmail.com> - 2015-09-21 17:10 +0200
                      Re: [PATCH 07/14] RFC: usb/host/fotg210: Remove a macro from snprintf Felipe Balbi <balbi@ti.com> - 2015-10-02 19:40 +0200
                    Re: [PATCH 01/14] RFC: usb/host/fotg210: Fix coding style issues Felipe Balbi <balbi@ti.com> - 2015-10-02 19:30 +0200
                    Re: [PATCH 12/14] RFC: usb/host/faraday-hcd: Replace fotg210 by  fhcd2xx Felipe Balbi <balbi@ti.com> - 2015-10-02 19:40 +0200
                    Re: [PATCH 11/14] RFC: usb/host: Rename fotg210-hcd to faraday-hcd Felipe Balbi <balbi@ti.com> - 2015-10-02 19:40 +0200
                    Re: [PATCH] usb-host: Remove fusbh200 driver Felipe Balbi <balbi@ti.com> - 2015-10-02 19:50 +0200
                      Re: [PATCH] usb-host: Remove fusbh200 driver Peter Senna Tschudin <peter.senna@gmail.com> - 2015-10-03 12:00 +0200
                        Re: [PATCH] usb-host: Remove fusbh200 driver Felipe Balbi <balbi@ti.com> - 2015-10-03 23:30 +0200
                          RE: [PATCH] usb-host: Remove fusbh200 driver John Feng-Hsin Chiang(江峰興)   <john453@faraday-tech.com> - 2015-10-05 02:50 +0200
                  Re: similar files: fusbh200-hcd.c and fotg210-hcd.c Peter Senna Tschudin <peter.senna@gmail.com> - 2015-09-25 15:10 +0200

Page 1 of 3  [1] 2 3  Next page →

#1220250 — similar files: fusbh200-hcd.c and fotg210-hcd.c

I executed a clone detection tool* on drivers source code and I found
that the files

drivers/usb/host/fusbh200-hcd.c

and

drivers/usb/host/fotg210-hcd.c

are very similar. The main difference between the two files are
replacing the string 'USBH20' by 'OTG21' and some white space fixes.
Some changes are being applied to only one of the files, such as the
commit f848a88d223cafa43cb318839a1171b498cf5ec8 that changes
fotg210-hcd.c but not fusbh200-hcd.c.

Should these files be consolidated? And if so how?

Thank you,

Peter

* https://github.com/petersenna/ccfinderx-core

-- 
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1220945

[Multipart message — attachments visible in raw view] — view raw

On Mon, Sep 07, 2015 at 04:47:45PM +0200, Peter Senna Tschudin wrote:
> I executed a clone detection tool* on drivers source code and I found
> that the files
> 
> drivers/usb/host/fusbh200-hcd.c
> 
> and
> 
> drivers/usb/host/fotg210-hcd.c
> 
> are very similar. The main difference between the two files are
> replacing the string 'USBH20' by 'OTG21' and some white space fixes.
> Some changes are being applied to only one of the files, such as the
> commit f848a88d223cafa43cb318839a1171b498cf5ec8 that changes
> fotg210-hcd.c but not fusbh200-hcd.c.
> 
> Should these files be consolidated? And if so how?

if you can find an easy way, that would be a very, very welcome patch.

-- 
balbi

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

#1223411

>> Should these files be consolidated? And if so how?
> if you can find an easy way, that would be a very, very welcome patch.

Is the ideal solution to consolidate both fusbh200-hcd.c and
fotg210-hcd.c in a single module? If this is the case, how to detect
at run time which version of the hw is present? Both are registered as
platform devices and I could not find an obvious way to detect the
model at run time. I could successfully load fusbh200-hcd on my fedora
notebook (hp elitebook 840), and on a VM, even if neither has the hw
($ sudo modprobe fusbh200-hcd). The module loads with the warning
"fusbh200_hcd should always be loaded before uhci_hcd and ohci_hcd,
not after". On another workstation running ubuntu, I could load both
modules at the same time, producing the same warning for each module.
Should the module load if the device is not present?

Other solution for consolidation would be to create a common_code.c,
keeping both fusbh200-hcd.c and fotg210-hcd.c only with the code that
differ. Is this better than what is there now?

Other ideas?


-- 
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1224222

[Multipart message — attachments visible in raw view] — view raw

On Sat, Sep 12, 2015 at 03:14:50PM +0200, Peter Senna Tschudin wrote:
> >> Should these files be consolidated? And if so how?
> > if you can find an easy way, that would be a very, very welcome patch.
> 
> Is the ideal solution to consolidate both fusbh200-hcd.c and
> fotg210-hcd.c in a single module? If this is the case, how to detect
> at run time which version of the hw is present? Both are registered as

does it matter ? If they work the same way, why does it matter which
one's running?

> platform devices and I could not find an obvious way to detect the
> model at run time. I could successfully load fusbh200-hcd on my fedora

is there a revision register ? Or you may use different platform_device
names with platform_device_id table.

> notebook (hp elitebook 840), and on a VM, even if neither has the hw
> ($ sudo modprobe fusbh200-hcd). The module loads with the warning
> "fusbh200_hcd should always be loaded before uhci_hcd and ohci_hcd,
> not after". On another workstation running ubuntu, I could load both
> modules at the same time, producing the same warning for each module.
> Should the module load if the device is not present?
> 
> Other solution for consolidation would be to create a common_code.c,
> keeping both fusbh200-hcd.c and fotg210-hcd.c only with the code that
> differ. Is this better than what is there now?
> 
> Other ideas?

just combine them :-p Use platform_device_id to differentiate.

-- 
balbi

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

#1224379

On Mon, Sep 14, 2015 at 5:01 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Sat, Sep 12, 2015 at 03:14:50PM +0200, Peter Senna Tschudin wrote:
>> >> Should these files be consolidated? And if so how?
>> > if you can find an easy way, that would be a very, very welcome patch.
>>
>> Is the ideal solution to consolidate both fusbh200-hcd.c and
>> fotg210-hcd.c in a single module? If this is the case, how to detect
>> at run time which version of the hw is present? Both are registered as
>
> does it matter ? If they work the same way, why does it matter which
> one's running?

I may be missing something simple, but based on a 2 page product
brief, fotg210 has more resources like memory. So even if the .c files
are _very_ similar, there are some configuration parameters that
differ, for example:

fusbh200.h:
#define BMCSR_VBUS_OFF (1<<4)
#define BMCSR_INT_POLARITY (1<<3)

fotg210.h:
#define OTGCSR_A_BUS_DROP (1 << 5)
#define OTGCSR_A_BUS_REQ (1 << 4)

which are used by {fusbh200,fotg210}_init:

fusbh200-hcd.c:
static void fusbh200_init(struct fusbh200_hcd *fusbh200)
{
u32 reg;

reg = fusbh200_readl(fusbh200, &fusbh200->regs->bmcsr);
reg |= BMCSR_INT_POLARITY;
reg &= ~BMCSR_VBUS_OFF;
fusbh200_writel(fusbh200, reg, &fusbh200->regs->bmcsr);

reg = fusbh200_readl(fusbh200, &fusbh200->regs->bmier);
fusbh200_writel(fusbh200, reg | BMIER_OVC_EN | BMIER_VBUS_ERR_EN,
&fusbh200->regs->bmier);
}


fotg210-hcd.c:
static void fotg210_init(struct fotg210_hcd *fotg210)
{
u32 value;

iowrite32(GMIR_MDEV_INT | GMIR_MOTG_INT | GMIR_INT_POLARITY,
 &fotg210->regs->gmir);

value = ioread32(&fotg210->regs->otgcsr);
value &= ~OTGCSR_A_BUS_DROP;
value |= OTGCSR_A_BUS_REQ;
iowrite32(value, &fotg210->regs->otgcsr);
}

Then:

fusbh200.h:
#define BMCSR_HOST_SPD_TYP (3<<9)

static inline unsigned int
fusbh200_get_speed(struct fusbh200_hcd *fusbh200, unsigned int portsc)
{
return (readl(&fusbh200->regs->bmcsr)
& BMCSR_HOST_SPD_TYP) >> 9;
}

fotg210.h:
#define OTGCSR_HOST_SPD_TYP     (3 << 22)
static inline unsigned int

fotg210_get_speed(struct fotg210_hcd *fotg210, unsigned int portsc)
{
return (readl(&fotg210->regs->otgcsr)
& OTGCSR_HOST_SPD_TYP) >> 22;
}

So my concern is to have a way to identify which is the device version
to use the right parameters. I think that the BMCSR_HOST_SPD_TYP vs
OTGCSR_HOST_SPD_TYP can be solved, but I'm not sure about the
initialization. Ideas?

>
>> platform devices and I could not find an obvious way to detect the
>> model at run time. I could successfully load fusbh200-hcd on my fedora
>
> is there a revision register ? Or you may use different platform_device
> names with platform_device_id table.

I don't know about revision registers. That would be good. I could not
find complete datasheets, only a 2 page product brief, no registry
information there.

>
>> notebook (hp elitebook 840), and on a VM, even if neither has the hw
>> ($ sudo modprobe fusbh200-hcd). The module loads with the warning
>> "fusbh200_hcd should always be loaded before uhci_hcd and ohci_hcd,
>> not after". On another workstation running ubuntu, I could load both
>> modules at the same time, producing the same warning for each module.
>> Should the module load if the device is not present?
>>
>> Other solution for consolidation would be to create a common_code.c,
>> keeping both fusbh200-hcd.c and fotg210-hcd.c only with the code that
>> differ. Is this better than what is there now?
>>
>> Other ideas?
>
> just combine them :-p Use platform_device_id to differentiate.

I'm afraid the combined version will use the correct parameters for
only one of the two. But I may be missing something simple. I did a
diff between the two files after removing white space differences, and
after replacing fusbh200 by fotg210 on the fusbh200 driver. The files
are very similar. See: http://pastebin.com/ZRY3xePv



>
> --
> balbi



-- 
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1225226

[Multipart message — attachments visible in raw view] — view raw

On Mon, Sep 14, 2015 at 07:50:02PM +0200, Peter Senna Tschudin wrote:
> On Mon, Sep 14, 2015 at 5:01 PM, Felipe Balbi <balbi@ti.com> wrote:
> > On Sat, Sep 12, 2015 at 03:14:50PM +0200, Peter Senna Tschudin wrote:
> >> >> Should these files be consolidated? And if so how?
> >> > if you can find an easy way, that would be a very, very welcome patch.
> >>
> >> Is the ideal solution to consolidate both fusbh200-hcd.c and
> >> fotg210-hcd.c in a single module? If this is the case, how to detect
> >> at run time which version of the hw is present? Both are registered as
> >
> > does it matter ? If they work the same way, why does it matter which
> > one's running?
> 
> I may be missing something simple, but based on a 2 page product
> brief, fotg210 has more resources like memory. So even if the .c files
> are _very_ similar, there are some configuration parameters that
> differ, for example:
> 
> fusbh200.h:
> #define BMCSR_VBUS_OFF (1<<4)
> #define BMCSR_INT_POLARITY (1<<3)
> 
> fotg210.h:
> #define OTGCSR_A_BUS_DROP (1 << 5)
> #define OTGCSR_A_BUS_REQ (1 << 4)

Can you detect that in runtime ? If you can, detect it. If you can't use
different platform_device_id.

> >> notebook (hp elitebook 840), and on a VM, even if neither has the hw
> >> ($ sudo modprobe fusbh200-hcd). The module loads with the warning
> >> "fusbh200_hcd should always be loaded before uhci_hcd and ohci_hcd,
> >> not after". On another workstation running ubuntu, I could load both
> >> modules at the same time, producing the same warning for each module.
> >> Should the module load if the device is not present?
> >>
> >> Other solution for consolidation would be to create a common_code.c,
> >> keeping both fusbh200-hcd.c and fotg210-hcd.c only with the code that
> >> differ. Is this better than what is there now?
> >>
> >> Other ideas?
> >
> > just combine them :-p Use platform_device_id to differentiate.
> 
> I'm afraid the combined version will use the correct parameters for
> only one of the two. But I may be missing something simple. I did a
> diff between the two files after removing white space differences, and
> after replacing fusbh200 by fotg210 on the fusbh200 driver. The files
> are very similar. See: http://pastebin.com/ZRY3xePv

yeah, you can totally combine them. Grep the tree for examples of how to
use platform_device_id as I mentioned.

-- 
balbi

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

#1225422

On Tue, Sep 15, 2015 at 4:33 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Mon, Sep 14, 2015 at 07:50:02PM +0200, Peter Senna Tschudin wrote:
>> On Mon, Sep 14, 2015 at 5:01 PM, Felipe Balbi <balbi@ti.com> wrote:
>> > On Sat, Sep 12, 2015 at 03:14:50PM +0200, Peter Senna Tschudin wrote:
>> >> >> Should these files be consolidated? And if so how?
>> >> > if you can find an easy way, that would be a very, very welcome patch.
>> >>
>> >> Is the ideal solution to consolidate both fusbh200-hcd.c and
>> >> fotg210-hcd.c in a single module? If this is the case, how to detect
>> >> at run time which version of the hw is present? Both are registered as
>> >
>> > does it matter ? If they work the same way, why does it matter which
>> > one's running?
>>
>> I may be missing something simple, but based on a 2 page product
>> brief, fotg210 has more resources like memory. So even if the .c files
>> are _very_ similar, there are some configuration parameters that
>> differ, for example:
>>
>> fusbh200.h:
>> #define BMCSR_VBUS_OFF (1<<4)
>> #define BMCSR_INT_POLARITY (1<<3)
>>
>> fotg210.h:
>> #define OTGCSR_A_BUS_DROP (1 << 5)
>> #define OTGCSR_A_BUS_REQ (1 << 4)
>
> Can you detect that in runtime ? If you can, detect it. If you can't use
> different platform_device_id.
>
>> >> notebook (hp elitebook 840), and on a VM, even if neither has the hw
>> >> ($ sudo modprobe fusbh200-hcd). The module loads with the warning
>> >> "fusbh200_hcd should always be loaded before uhci_hcd and ohci_hcd,
>> >> not after". On another workstation running ubuntu, I could load both
>> >> modules at the same time, producing the same warning for each module.
>> >> Should the module load if the device is not present?
>> >>
>> >> Other solution for consolidation would be to create a common_code.c,
>> >> keeping both fusbh200-hcd.c and fotg210-hcd.c only with the code that
>> >> differ. Is this better than what is there now?
>> >>
>> >> Other ideas?
>> >
>> > just combine them :-p Use platform_device_id to differentiate.

Can you check the f2xx branch at:

git@github.com:petersenna/linux.git

And tell me if this is the way to go for the consolidation of the two
drivers? I started with the newest driver, did code cleanup, and
started filling the new driver with parameters from the older
FUSBH200. At the moment it compiles for x86 and probably still works
for FOTG210 devices. A concrete question I have is if should I keep
making many patches for the consolidation or should I do a single big
patch with all changes? Comments are welcome.

>>
>> I'm afraid the combined version will use the correct parameters for
>> only one of the two. But I may be missing something simple. I did a
>> diff between the two files after removing white space differences, and
>> after replacing fusbh200 by fotg210 on the fusbh200 driver. The files
>> are very similar. See: http://pastebin.com/ZRY3xePv
>
> yeah, you can totally combine them. Grep the tree for examples of how to
> use platform_device_id as I mentioned.

I'll do it. Thank you.

>
> --
> balbi



-- 
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1225434

[Multipart message — attachments visible in raw view] — view raw

Hi,

On Tue, Sep 15, 2015 at 06:41:55PM +0200, Peter Senna Tschudin wrote:
> On Tue, Sep 15, 2015 at 4:33 PM, Felipe Balbi <balbi@ti.com> wrote:
> > On Mon, Sep 14, 2015 at 07:50:02PM +0200, Peter Senna Tschudin wrote:
> >> On Mon, Sep 14, 2015 at 5:01 PM, Felipe Balbi <balbi@ti.com> wrote:
> >> > On Sat, Sep 12, 2015 at 03:14:50PM +0200, Peter Senna Tschudin wrote:
> >> >> >> Should these files be consolidated? And if so how?
> >> >> > if you can find an easy way, that would be a very, very welcome patch.
> >> >>
> >> >> Is the ideal solution to consolidate both fusbh200-hcd.c and
> >> >> fotg210-hcd.c in a single module? If this is the case, how to detect
> >> >> at run time which version of the hw is present? Both are registered as
> >> >
> >> > does it matter ? If they work the same way, why does it matter which
> >> > one's running?
> >>
> >> I may be missing something simple, but based on a 2 page product
> >> brief, fotg210 has more resources like memory. So even if the .c files
> >> are _very_ similar, there are some configuration parameters that
> >> differ, for example:
> >>
> >> fusbh200.h:
> >> #define BMCSR_VBUS_OFF (1<<4)
> >> #define BMCSR_INT_POLARITY (1<<3)
> >>
> >> fotg210.h:
> >> #define OTGCSR_A_BUS_DROP (1 << 5)
> >> #define OTGCSR_A_BUS_REQ (1 << 4)
> >
> > Can you detect that in runtime ? If you can, detect it. If you can't use
> > different platform_device_id.
> >
> >> >> notebook (hp elitebook 840), and on a VM, even if neither has the hw
> >> >> ($ sudo modprobe fusbh200-hcd). The module loads with the warning
> >> >> "fusbh200_hcd should always be loaded before uhci_hcd and ohci_hcd,
> >> >> not after". On another workstation running ubuntu, I could load both
> >> >> modules at the same time, producing the same warning for each module.
> >> >> Should the module load if the device is not present?
> >> >>
> >> >> Other solution for consolidation would be to create a common_code.c,
> >> >> keeping both fusbh200-hcd.c and fotg210-hcd.c only with the code that
> >> >> differ. Is this better than what is there now?
> >> >>
> >> >> Other ideas?
> >> >
> >> > just combine them :-p Use platform_device_id to differentiate.
> 
> Can you check the f2xx branch at:
> 
> git@github.com:petersenna/linux.git
> 
> And tell me if this is the way to go for the consolidation of the two
> drivers? I started with the newest driver, did code cleanup, and
> started filling the new driver with parameters from the older
> FUSBH200. At the moment it compiles for x86 and probably still works
> for FOTG210 devices. A concrete question I have is if should I keep
> making many patches for the consolidation or should I do a single big
> patch with all changes? Comments are welcome.

it's best to just send patches. Also, you gave me an ssh URL which I
can't use because I don't have write access to your tree (and I don't
want to have it).

-- 
balbi

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

#1229449 — [PATCH 08/14] RFC: usb/host/fotg210: convert macro to inline function

This patch convert the macro speed_char in an inline function. The goal
of this patch is to make the code easier to read.

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
---
 drivers/usb/host/fotg210-hcd.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c
index 4032ed0..82cd5da 100644
--- a/drivers/usb/host/fotg210-hcd.c
+++ b/drivers/usb/host/fotg210-hcd.c
@@ -331,17 +331,22 @@ struct debug_buffer {
 	size_t alloc_size;
 };
 
-#define speed_char(info1)({ char tmp; \
-		switch (info1 & (3 << 12)) { \
-		case QH_FULL_SPEED:	\
-			tmp = 'f'; break; \
-		case QH_LOW_SPEED:	\
-			tmp = 'l'; break; \
-		case QH_HIGH_SPEED:	\
-			tmp = 'h'; break; \
-		default:		\
-			tmp = '?'; break; \
-		} tmp; })
+static inline char speed_char(u32 scratch)
+{
+	switch (scratch & (3 << 12)) {
+	case QH_FULL_SPEED:
+		return 'f';
+
+	case QH_LOW_SPEED:
+		return 'l';
+
+	case QH_HIGH_SPEED:
+		return 'h';
+
+	default:
+		return '?';
+	}
+}
 
 static inline char token_mark(struct fotg210_hcd *fotg210, __hc32 token)
 {
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1238446 — Re: [PATCH 08/14] RFC: usb/host/fotg210: convert macro to inline function

[Multipart message — attachments visible in raw view] — view raw

On Mon, Sep 21, 2015 at 05:01:12PM +0200, Peter Senna Tschudin wrote:
> This patch convert the macro speed_char in an inline function. The goal
> of this patch is to make the code easier to read.
> 
> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>

looks good

> ---
>  drivers/usb/host/fotg210-hcd.c | 27 ++++++++++++++++-----------
>  1 file changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c
> index 4032ed0..82cd5da 100644
> --- a/drivers/usb/host/fotg210-hcd.c
> +++ b/drivers/usb/host/fotg210-hcd.c
> @@ -331,17 +331,22 @@ struct debug_buffer {
>  	size_t alloc_size;
>  };
>  
> -#define speed_char(info1)({ char tmp; \
> -		switch (info1 & (3 << 12)) { \
> -		case QH_FULL_SPEED:	\
> -			tmp = 'f'; break; \
> -		case QH_LOW_SPEED:	\
> -			tmp = 'l'; break; \
> -		case QH_HIGH_SPEED:	\
> -			tmp = 'h'; break; \
> -		default:		\
> -			tmp = '?'; break; \
> -		} tmp; })
> +static inline char speed_char(u32 scratch)
> +{
> +	switch (scratch & (3 << 12)) {
> +	case QH_FULL_SPEED:
> +		return 'f';
> +
> +	case QH_LOW_SPEED:
> +		return 'l';
> +
> +	case QH_HIGH_SPEED:
> +		return 'h';
> +
> +	default:
> +		return '?';
> +	}
> +}
>  
>  static inline char token_mark(struct fotg210_hcd *fotg210, __hc32 token)
>  {
> -- 
> 2.1.0
> 

-- 
balbi

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

#1229451 — [PATCH 09/14] RFC: usb/host/fotg210: Add function: output_buf_tds_dir()

checkpatch complains about too many leading tabs because the switch
statement starts after 6 tabs.

fill_periodic_buffer() -> for() -> do -> switch() -> if() ->
list_for_each_entry() and finally the last switch().

This patch moves the list_for_each_entry() and the last switch() to a
new inline function named output_buf_tds_dir(). This change makes the
code easier to read and calm down checkpatch. This patch changes it to:

fill_periodic_buffer() -> for() -> do -> switch() -> if() ->
output_buf_tds_dir()

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
---
 drivers/usb/host/fotg210-hcd.c | 64 ++++++++++++++++++++++--------------------
 1 file changed, 33 insertions(+), 31 deletions(-)

diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c
index 82cd5da..13cca41 100644
--- a/drivers/usb/host/fotg210-hcd.c
+++ b/drivers/usb/host/fotg210-hcd.c
@@ -497,6 +497,36 @@ static ssize_t fill_async_buffer(struct debug_buffer *buf)
 	return strlen(buf->output_buf);
 }
 
+/* count tds, get ep direction */
+static inline unsigned output_buf_tds_dir(char *buf,
+					  struct fotg210_hcd *fotg210,
+					  struct fotg210_qh_hw *hw,
+					  struct fotg210_qh *qh, unsigned size)
+{
+	u32 scratch = hc32_to_cpup(fotg210, &hw->hw_info1);
+	struct fotg210_qtd *qtd;
+	char *type = "";
+	unsigned temp = 0;
+
+	/* count tds, get ep direction */
+	list_for_each_entry(qtd, &qh->qtd_list, qtd_list) {
+		temp++;
+		switch (0x03 & (hc32_to_cpu(fotg210, qtd->hw_token) >> 8)) {
+		case 0:
+			type = "out";
+			continue;
+		case 1:
+			type = "in";
+			continue;
+		}
+	}
+
+	return scnprintf(buf, size, "(%c%d ep%d%s [%d/%d] q%d p%d)",
+			 speed_char(scratch), scratch & 0x007f,
+			 (scratch >> 8) & 0x000f, type, qh->usecs,
+			 qh->c_usecs, temp, 0x7ff & (scratch >> 16));
+}
+
 #define DBG_SCHED_LIMIT 64
 static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
 {
@@ -568,37 +598,9 @@ static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
 				}
 				/* show more info the first time around */
 				if (temp == seen_count) {
-					u32 scratch = hc32_to_cpup(fotg210,
-								&hw->hw_info1);
-					struct fotg210_qtd *qtd;
-					char *type = "";
-
-					/* count tds, get ep direction */
-					temp = 0;
-					list_for_each_entry(qtd,
-							&p.qh->qtd_list,
-							qtd_list) {
-						temp++;
-						switch (0x03 & (hc32_to_cpu(
-							fotg210,
-							qtd->hw_token) >> 8)) {
-						case 0:
-							type = "out";
-							continue;
-						case 1:
-							type = "in";
-							continue;
-						}
-					}
-
-					temp = scnprintf(next, size,
-						"(%c%d ep%d%s [%d/%d] q%d p%d)",
-						speed_char(scratch),
-						scratch & 0x007f,
-						(scratch >> 8) & 0x000f, type,
-						p.qh->usecs, p.qh->c_usecs,
-						temp,
-						0x7ff & (scratch >> 16));
+					temp = output_buf_tds_dir(next,
+								  fotg210, hw,
+								  p.qh, size);
 
 					if (seen_count < DBG_SCHED_LIMIT)
 						seen[seen_count++].qh = p.qh;
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1229678 — Re: [PATCH 09/14] RFC: usb/host/fotg210: Add function: output_buf_tds_dir()

On 09/21/2015 06:01 PM, Peter Senna Tschudin wrote:

> checkpatch complains about too many leading tabs because the switch
> statement starts after 6 tabs.
>
> fill_periodic_buffer() -> for() -> do -> switch() -> if() ->
> list_for_each_entry() and finally the last switch().
>
> This patch moves the list_for_each_entry() and the last switch() to a
> new inline function named output_buf_tds_dir(). This change makes the
> code easier to read and calm down checkpatch. This patch changes it to:
>
> fill_periodic_buffer() -> for() -> do -> switch() -> if() ->
> output_buf_tds_dir()
>
> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
> ---
>   drivers/usb/host/fotg210-hcd.c | 64 ++++++++++++++++++++++--------------------
>   1 file changed, 33 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c
> index 82cd5da..13cca41 100644
> --- a/drivers/usb/host/fotg210-hcd.c
> +++ b/drivers/usb/host/fotg210-hcd.c
> @@ -497,6 +497,36 @@ static ssize_t fill_async_buffer(struct debug_buffer *buf)
>   	return strlen(buf->output_buf);
>   }
>
> +/* count tds, get ep direction */
> +static inline unsigned output_buf_tds_dir(char *buf,

    Please drop *inline*, let gcc figure it out.

> +					  struct fotg210_hcd *fotg210,
> +					  struct fotg210_qh_hw *hw,
> +					  struct fotg210_qh *qh, unsigned size)
> +{
> +	u32 scratch = hc32_to_cpup(fotg210, &hw->hw_info1);
> +	struct fotg210_qtd *qtd;
> +	char *type = "";
> +	unsigned temp = 0;
> +
> +	/* count tds, get ep direction */
> +	list_for_each_entry(qtd, &qh->qtd_list, qtd_list) {
> +		temp++;
> +		switch (0x03 & (hc32_to_cpu(fotg210, qtd->hw_token) >> 8)) {

    I think the preference is to place immediate value after &.

> +		case 0:
> +			type = "out";
> +			continue;
> +		case 1:
> +			type = "in";
> +			continue;
> +		}
> +	}
> +
> +	return scnprintf(buf, size, "(%c%d ep%d%s [%d/%d] q%d p%d)",
> +			 speed_char(scratch), scratch & 0x007f,
> +			 (scratch >> 8) & 0x000f, type, qh->usecs,
> +			 qh->c_usecs, temp, 0x7ff & (scratch >> 16));

    Likewise.

[...]

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1238445 — Re: [PATCH 09/14] RFC: usb/host/fotg210: Add function: output_buf_tds_dir()

[Multipart message — attachments visible in raw view] — view raw

On Mon, Sep 21, 2015 at 05:01:13PM +0200, Peter Senna Tschudin wrote:
> checkpatch complains about too many leading tabs because the switch
> statement starts after 6 tabs.
> 
> fill_periodic_buffer() -> for() -> do -> switch() -> if() ->
> list_for_each_entry() and finally the last switch().
> 
> This patch moves the list_for_each_entry() and the last switch() to a
> new inline function named output_buf_tds_dir(). This change makes the
> code easier to read and calm down checkpatch. This patch changes it to:
> 
> fill_periodic_buffer() -> for() -> do -> switch() -> if() ->
> output_buf_tds_dir()
> 
> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>

if you fix Sergei's comment, then I'm okay with $subject

> ---
>  drivers/usb/host/fotg210-hcd.c | 64 ++++++++++++++++++++++--------------------
>  1 file changed, 33 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c
> index 82cd5da..13cca41 100644
> --- a/drivers/usb/host/fotg210-hcd.c
> +++ b/drivers/usb/host/fotg210-hcd.c
> @@ -497,6 +497,36 @@ static ssize_t fill_async_buffer(struct debug_buffer *buf)
>  	return strlen(buf->output_buf);
>  }
>  
> +/* count tds, get ep direction */
> +static inline unsigned output_buf_tds_dir(char *buf,
> +					  struct fotg210_hcd *fotg210,
> +					  struct fotg210_qh_hw *hw,
> +					  struct fotg210_qh *qh, unsigned size)
> +{
> +	u32 scratch = hc32_to_cpup(fotg210, &hw->hw_info1);
> +	struct fotg210_qtd *qtd;
> +	char *type = "";
> +	unsigned temp = 0;
> +
> +	/* count tds, get ep direction */
> +	list_for_each_entry(qtd, &qh->qtd_list, qtd_list) {
> +		temp++;
> +		switch (0x03 & (hc32_to_cpu(fotg210, qtd->hw_token) >> 8)) {
> +		case 0:
> +			type = "out";
> +			continue;
> +		case 1:
> +			type = "in";
> +			continue;
> +		}
> +	}
> +
> +	return scnprintf(buf, size, "(%c%d ep%d%s [%d/%d] q%d p%d)",
> +			 speed_char(scratch), scratch & 0x007f,
> +			 (scratch >> 8) & 0x000f, type, qh->usecs,
> +			 qh->c_usecs, temp, 0x7ff & (scratch >> 16));
> +}
> +
>  #define DBG_SCHED_LIMIT 64
>  static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
>  {
> @@ -568,37 +598,9 @@ static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
>  				}
>  				/* show more info the first time around */
>  				if (temp == seen_count) {
> -					u32 scratch = hc32_to_cpup(fotg210,
> -								&hw->hw_info1);
> -					struct fotg210_qtd *qtd;
> -					char *type = "";
> -
> -					/* count tds, get ep direction */
> -					temp = 0;
> -					list_for_each_entry(qtd,
> -							&p.qh->qtd_list,
> -							qtd_list) {
> -						temp++;
> -						switch (0x03 & (hc32_to_cpu(
> -							fotg210,
> -							qtd->hw_token) >> 8)) {
> -						case 0:
> -							type = "out";
> -							continue;
> -						case 1:
> -							type = "in";
> -							continue;
> -						}
> -					}
> -
> -					temp = scnprintf(next, size,
> -						"(%c%d ep%d%s [%d/%d] q%d p%d)",
> -						speed_char(scratch),
> -						scratch & 0x007f,
> -						(scratch >> 8) & 0x000f, type,
> -						p.qh->usecs, p.qh->c_usecs,
> -						temp,
> -						0x7ff & (scratch >> 16));
> +					temp = output_buf_tds_dir(next,
> +								  fotg210, hw,
> +								  p.qh, size);
>  
>  					if (seen_count < DBG_SCHED_LIMIT)
>  						seen[seen_count++].qh = p.qh;
> -- 
> 2.1.0
> 

-- 
balbi

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

#1229453 — [PATCH 14/14] RFC: usb/host/faraday-hcd: Import FUSBH200 parameters

This patch adds FUSBH200 parameters to faraday-hcd.h.

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
---
 drivers/usb/host/faraday-hcd.h | 50 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/faraday-hcd.h b/drivers/usb/host/faraday-hcd.h
index f75c467..fd3b261 100644
--- a/drivers/usb/host/faraday-hcd.h
+++ b/drivers/usb/host/faraday-hcd.h
@@ -259,6 +259,14 @@ struct fhcd2xx_caps {
 #define PORT_CONNECT	(1<<0) /* device connected */
 #define PORT_RWC_BITS   (PORT_CSC | PORT_PEC)
 
+/* FUSB200 only: */
+#define BMCSR_HOST_SPD_TYP      (3<<9)
+#define BMCSR_VBUS_OFF          (1<<4)
+#define BMCSR_INT_POLARITY      (1<<3)
+#define BMISR_OVC               (1<<1)
+#define BMIER_OVC_EN            (1<<1)
+#define BMIER_VBUS_ERR_EN       (1<<0)
+
 /* FOTG210 only: */
 #define OTGCSR_HOST_SPD_TYP     (3 << 22)
 #define OTGCSR_A_BUS_DROP	(1 << 5)
@@ -269,7 +277,47 @@ struct fhcd2xx_caps {
 #define GMIR_MOTG_INT		(1 << 1)
 #define GMIR_MDEV_INT		(1 << 0)
 
-/* Section 2.3 Host Controller Operational Registers */
+/* FUSB200: Section 2.3 Host Controller Operational Registers */
+struct fusbh200_regs {
+	/* USBCMD: offset 0x00 */
+	u32 command;
+
+	/* USBSTS: offset 0x04 */
+	u32 status;
+
+	/* USBINTR: offset 0x08 */
+	u32 intr_enable;
+
+	/* FRINDEX: offset 0x0C - current microframe number */
+	u32 frame_index;
+
+	/* CTRLDSSEGMENT: offset 0x10 - address bits 63:32 if needed */
+	u32 segment;
+
+	/* PERIODICLISTBASE: offset 0x14 - points to periodic list */
+	u32 frame_list;
+
+	/* ASYNCLISTADDR: offset 0x18 - address of next async queue head */
+	u32 async_next;
+
+	u32 reserved1;
+
+	/* PORTSC: offset 0x20 */
+	u32 port_status;
+
+	u32 reserved2[3];
+
+	/* BMCSR: offset 0x30 - Bus Moniter Control/Status Register */
+	u32 bmcsr;
+
+	/* BMISR: offset 0x34 - Bus Moniter Interrupt Status Register */
+	u32 bmisr;
+
+	/* BMIER: offset 0x38 - Bus Moniter Interrupt Enable Register */
+	u32 bmier;
+};
+
+/* FOTG210: Section 2.3 Host Controller Operational Registers */
 struct fotg210_regs {
 
 	/* USBCMD: offset 0x00 */
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1238454 — Re: [PATCH 14/14] RFC: usb/host/faraday-hcd: Import FUSBH200 parameters

[Multipart message — attachments visible in raw view] — view raw

On Mon, Sep 21, 2015 at 05:01:18PM +0200, Peter Senna Tschudin wrote:
> This patch adds FUSBH200 parameters to faraday-hcd.h.
> 
> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>

fine by me


-- 
balbi

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

#1229455 — [PATCH 00/14] RFC: Consolidation: FUSB200 and FOTG210

I'm working on the consolidation of two very similar Faraday EHCI drivers:

drivers/usb/host/fusbh200-hcd.c and drivers/usb/host/fotg210-hcd.c

I'm sending the work unfinished because I have some questions, and I would
appreciate feedback about the changes. As this is not complete work, please do
_not_ apply the series. 

Patches from 01 to 10 are fixes to issues reported by checkpatch, patches 11,
12 and 13 are changes to prepare the code to support multiple devices, and
patch 14 adds parameters from fusb200 into faraday-hcd.

My questions:
 - Is the name faraday-hcd good for the driver that will support both
   fusb200-hcd and fotg210-hcd?

 - Is this patch series in a good direction for consolidating the two drivers?
   I mean it is a lot of patches, and there are many different orders in which
   I could send them.

 - Is it a good idea to include now changes like patches 06, 09 and 10 that may
   have impact on performance? Or should I wait until I can get some hw to
   test?

 - Do you know easy to find and cheap devices using this two host controllers?

 - Should I worry about drivers/usb/gadget/udc/fotg210-udc.c?

Peter Senna Tschudin (14):
  usb/host/fotg210: Fix coding style issues
  usb/host/fotg210: remove KERN_WARNING from pr_info
  usb/host/fotg210: Remove useless else statement
  usb/host/fotg210: Remove NULL checks dma_pool_destroy
  usb/host/fotg210: change kmalloc by kmalloc_array
  usb/host/fotg210: replace msleep by usleep_range
  usb/host/fotg210: Remove a macro from a call to snprintf
  usb/host/fotg210: convert macro to inline function
  usb/host/fotg210: Add function: output_buf_tds_dir()
  usb/host/fotg210: Add function scan_frame_queue()
  usb/host: Rename fotg210-hcd to faraday-hcd
  usb/host/faraday-hcd: Replace fotg210 by fhcd2xx
  usb/host/faraday-hcd: Move #defines outside struct
  usb/host/faraday-hcd: Import FUSBH200 parameters

 drivers/usb/host/Kconfig       |    2 +-
 drivers/usb/host/Makefile      |    2 +-
 drivers/usb/host/faraday-hcd.c | 5792 ++++++++++++++++++++++++++++++++++++++
 drivers/usb/host/faraday-hcd.h |  751 +++++
 drivers/usb/host/fotg210-hcd.c | 5951 ----------------------------------------
 drivers/usb/host/fotg210.h     |  686 -----
 6 files changed, 6545 insertions(+), 6639 deletions(-)
 create mode 100644 drivers/usb/host/faraday-hcd.c
 create mode 100644 drivers/usb/host/faraday-hcd.h
 delete mode 100644 drivers/usb/host/fotg210-hcd.c
 delete mode 100644 drivers/usb/host/fotg210.h

-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1229457 — [PATCH 04/14] RFC: usb/host/fotg210: Remove NULL checks dma_pool_destroy

This patch remove NULL checks before calls to dma_pool_destroy() as the
function now can handle NULL pointers.

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
---
 drivers/usb/host/fotg210-hcd.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c
index f574143..51feb61 100644
--- a/drivers/usb/host/fotg210-hcd.c
+++ b/drivers/usb/host/fotg210-hcd.c
@@ -1925,17 +1925,13 @@ static void fotg210_mem_cleanup(struct fotg210_hcd *fotg210)
 	fotg210->dummy = NULL;
 
 	/* DMA consistent memory and pools */
-	if (fotg210->qtd_pool)
-		dma_pool_destroy(fotg210->qtd_pool);
+	dma_pool_destroy(fotg210->qtd_pool);
 	fotg210->qtd_pool = NULL;
 
-	if (fotg210->qh_pool) {
-		dma_pool_destroy(fotg210->qh_pool);
-		fotg210->qh_pool = NULL;
-	}
+	dma_pool_destroy(fotg210->qh_pool);
+	fotg210->qh_pool = NULL;
 
-	if (fotg210->itd_pool)
-		dma_pool_destroy(fotg210->itd_pool);
+	dma_pool_destroy(fotg210->itd_pool);
 	fotg210->itd_pool = NULL;
 
 	if (fotg210->periodic)
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1238460 — Re: [PATCH 04/14] RFC: usb/host/fotg210: Remove NULL checks dma_pool_destroy

[Multipart message — attachments visible in raw view] — view raw

On Mon, Sep 21, 2015 at 05:01:08PM +0200, Peter Senna Tschudin wrote:
> This patch remove NULL checks before calls to dma_pool_destroy() as the
> function now can handle NULL pointers.
> 
> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>

looks good to me

> ---
>  drivers/usb/host/fotg210-hcd.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c
> index f574143..51feb61 100644
> --- a/drivers/usb/host/fotg210-hcd.c
> +++ b/drivers/usb/host/fotg210-hcd.c
> @@ -1925,17 +1925,13 @@ static void fotg210_mem_cleanup(struct fotg210_hcd *fotg210)
>  	fotg210->dummy = NULL;
>  
>  	/* DMA consistent memory and pools */
> -	if (fotg210->qtd_pool)
> -		dma_pool_destroy(fotg210->qtd_pool);
> +	dma_pool_destroy(fotg210->qtd_pool);
>  	fotg210->qtd_pool = NULL;
>  
> -	if (fotg210->qh_pool) {
> -		dma_pool_destroy(fotg210->qh_pool);
> -		fotg210->qh_pool = NULL;
> -	}
> +	dma_pool_destroy(fotg210->qh_pool);
> +	fotg210->qh_pool = NULL;
>  
> -	if (fotg210->itd_pool)
> -		dma_pool_destroy(fotg210->itd_pool);
> +	dma_pool_destroy(fotg210->itd_pool);
>  	fotg210->itd_pool = NULL;
>  
>  	if (fotg210->periodic)
> -- 
> 2.1.0
> 

-- 
balbi

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

#1229458 — [PATCH 03/14] RFC: usb/host/fotg210: Remove useless else statement

This patch remove an else statement after a return to make the code
easier to understand.

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
---
 drivers/usb/host/fotg210-hcd.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c
index 36413b2..f574143 100644
--- a/drivers/usb/host/fotg210-hcd.c
+++ b/drivers/usb/host/fotg210-hcd.c
@@ -1410,10 +1410,9 @@ static int check_reset_complete(struct fotg210_hcd *fotg210, int index,
 			"Failed to enable port %d on root hub TT\n",
 			index+1);
 		return port_status;
-	} else {
-		fotg210_dbg(fotg210, "port %d reset complete, port enabled\n",
-			index + 1);
 	}
+	fotg210_dbg(fotg210, "port %d reset complete, port enabled\n",
+		    index + 1);
 
 	return port_status;
 }
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1238465 — Re: [PATCH 03/14] RFC: usb/host/fotg210: Remove useless else statement

[Multipart message — attachments visible in raw view] — view raw

On Mon, Sep 21, 2015 at 05:01:07PM +0200, Peter Senna Tschudin wrote:
> This patch remove an else statement after a return to make the code
> easier to understand.
> 
> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>

looks good to me.

> ---
>  drivers/usb/host/fotg210-hcd.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c
> index 36413b2..f574143 100644
> --- a/drivers/usb/host/fotg210-hcd.c
> +++ b/drivers/usb/host/fotg210-hcd.c
> @@ -1410,10 +1410,9 @@ static int check_reset_complete(struct fotg210_hcd *fotg210, int index,
>  			"Failed to enable port %d on root hub TT\n",
>  			index+1);
>  		return port_status;
> -	} else {
> -		fotg210_dbg(fotg210, "port %d reset complete, port enabled\n",
> -			index + 1);
>  	}
> +	fotg210_dbg(fotg210, "port %d reset complete, port enabled\n",
> +		    index + 1);
>  
>  	return port_status;
>  }
> -- 
> 2.1.0
> 

-- 
balbi

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

Page 1 of 3  [1] 2 3  Next page →

Back to top | Article view | linux.kernel

csiph-web