Patchwork [Precise,Pull-Request] Add realtek pcie card reader driver to fix #1057089

login
register
mail settings
Submitter Adam Lee
Date Jan. 28, 2013, 7:03 a.m.
Message ID <20130128070321.GC10628@adam-laptop>
Download mbox
Permalink /patch/216114/
State New
Headers show

Pull-request

git://kernel.ubuntu.com/adamlee/ubuntu-precise.git master

Comments

Adam Lee - Jan. 28, 2013, 7:03 a.m.
BugLink: http://launchpad.net/bugs/1057089

Backported the upstream realtek pcie card reader driver.

====

The following changes since commit 07daba920a41ed25fcfeee6fa7cf29c3073eae57:

  drivers/mfd: Add realtek pcie card reader driver (2013-01-28 14:07:45 +0800)

are available in the git repository at:

  git://kernel.ubuntu.com/adamlee/ubuntu-precise.git master

for you to fetch changes up to 2fe331e351e465e2b9d262cb5842c350ed139318:

  [Config] Enable RTSX_PCI modules (2013-01-28 14:07:49 +0800)

----------------------------------------------------------------
Adam Lee (1):
      [Config] Enable RTSX_PCI modules

Wei WANG (6):
      mmc: Add realtek pcie sdmmc host driver
      mmc: rtsx: Remove a duplicate command in sd_rw_multi
      mmc: rtsx: Configure SD_CFG2 register in sd_rw_multi
      mmc: rtsx: Explicitely include slab.h in rtsx_pci_sdmmc.c
      MFD:rtsx:Avoid kernel panic if rtsx_pci_sdmmc not probed
      drivers/memstick: Add realtek pcie memstick host driver

 debian.master/config/config.common.ubuntu |    3 +
 drivers/memstick/host/Kconfig             |   10 +
 drivers/memstick/host/Makefile            |    1 +
 drivers/memstick/host/rtsx_pci_ms.c       |  641 ++++++++++++++
 drivers/mfd/rtsx_pcr.c                    |    4 +-
 drivers/mmc/host/Kconfig                  |    7 +
 drivers/mmc/host/Makefile                 |    2 +
 drivers/mmc/host/rtsx_pci_sdmmc.c         | 1326 +++++++++++++++++++++++++++++
 8 files changed, 1992 insertions(+), 2 deletions(-)
 create mode 100644 drivers/memstick/host/rtsx_pci_ms.c
 create mode 100644 drivers/mmc/host/rtsx_pci_sdmmc.c
Luis Henriques - Jan. 28, 2013, 2:40 p.m.
Hi Adam,

On Mon, Jan 28, 2013 at 03:03:21PM +0800, Adam Lee wrote:
> BugLink: http://launchpad.net/bugs/1057089
> 
> Backported the upstream realtek pcie card reader driver.

I've took a quick look at these patches and I found a couple of issues:

Commits don't identify their origin -- are they upstream cherry-picks?
If so, they should contain the "(cherry-picked from commit XXXXXX)"
entry.  Also, they don't contain the BugLink.

I've tried to find the corresponding upstream commits and there's at
least one I couldn't find anywhere:

 commit 260b2cae1daca5640c0efcb9c93ccfc10b24b34e
 "MFD:rtsx:Avoid kernel panic if rtsx_pci_sdmmc not probed"

If this is not upstream, it should also contain 'UBUNTU: SAUCE' and an
indication of its source.

Finally, there are some changes made from the original upstream commits
that are difficult to understand and that may cause maintenance
problems.  For example, your commit
0b3359b47ca714c9bb6f617e495138b298093038 contains an extra function
(rtsx_pci_switch_output_voltage) that is not in the corresponding
upstream commit; however, there's another function in upstream commit
2c94b6452cc6de582584f21066cc5e36d9530c59 (your commit
7c6e81b68539422a1dee4f602f6a62ab402bdb01) that has function
sd_change_bank_voltage.  What's the rational behind this?  Can't we get
some clean cherry-picks?

Cheers,
--
Luis

> 
> ====
> 
> The following changes since commit 07daba920a41ed25fcfeee6fa7cf29c3073eae57:
> 
>   drivers/mfd: Add realtek pcie card reader driver (2013-01-28 14:07:45 +0800)
> 
> are available in the git repository at:
> 
>   git://kernel.ubuntu.com/adamlee/ubuntu-precise.git master
> 
> for you to fetch changes up to 2fe331e351e465e2b9d262cb5842c350ed139318:
> 
>   [Config] Enable RTSX_PCI modules (2013-01-28 14:07:49 +0800)
> 
> ----------------------------------------------------------------
> Adam Lee (1):
>       [Config] Enable RTSX_PCI modules
> 
> Wei WANG (6):
>       mmc: Add realtek pcie sdmmc host driver
>       mmc: rtsx: Remove a duplicate command in sd_rw_multi
>       mmc: rtsx: Configure SD_CFG2 register in sd_rw_multi
>       mmc: rtsx: Explicitely include slab.h in rtsx_pci_sdmmc.c
>       MFD:rtsx:Avoid kernel panic if rtsx_pci_sdmmc not probed
>       drivers/memstick: Add realtek pcie memstick host driver
> 
>  debian.master/config/config.common.ubuntu |    3 +
>  drivers/memstick/host/Kconfig             |   10 +
>  drivers/memstick/host/Makefile            |    1 +
>  drivers/memstick/host/rtsx_pci_ms.c       |  641 ++++++++++++++
>  drivers/mfd/rtsx_pcr.c                    |    4 +-
>  drivers/mmc/host/Kconfig                  |    7 +
>  drivers/mmc/host/Makefile                 |    2 +
>  drivers/mmc/host/rtsx_pci_sdmmc.c         | 1326 +++++++++++++++++++++++++++++
>  8 files changed, 1992 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/memstick/host/rtsx_pci_ms.c
>  create mode 100644 drivers/mmc/host/rtsx_pci_sdmmc.c
> 
> -- 
> Regards,
> Adam Lee
> Hardware Enablement
> IRC: adam8157 @ #china #hwe
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Adam Lee - Jan. 29, 2013, 3 a.m.
Hi, Luis,

On Mon, Jan 28, 2013 at 02:40:14PM +0000, Luis Henriques wrote:
> Hi Adam,
> 
> On Mon, Jan 28, 2013 at 03:03:21PM +0800, Adam Lee wrote:
> > BugLink: http://launchpad.net/bugs/1057089
> > 
> > Backported the upstream realtek pcie card reader driver.
> 
> I've took a quick look at these patches and I found a couple of issues:
> 
> Commits don't identify their origin -- are they upstream cherry-picks?
> If so, they should contain the "(cherry-picked from commit XXXXXX)"
> entry.  Also, they don't contain the BugLink.

Oops, sorry.

> I've tried to find the corresponding upstream commits and there's at
> least one I couldn't find anywhere:
> 
>  commit 260b2cae1daca5640c0efcb9c93ccfc10b24b34e
>  "MFD:rtsx:Avoid kernel panic if rtsx_pci_sdmmc not probed"
> 
> If this is not upstream, it should also contain 'UBUNTU: SAUCE' and an
> indication of its source.

This is not from upstream, but from LKML. This patch fixes a panic
issue, I think it should be merged in even it's not merged by upstream
mainline yet.
 
> Finally, there are some changes made from the original upstream commits
> that are difficult to understand and that may cause maintenance
> problems.  For example, your commit
> 0b3359b47ca714c9bb6f617e495138b298093038 contains an extra function
> (rtsx_pci_switch_output_voltage) that is not in the corresponding
> upstream commit; however, there's another function in upstream commit
> 2c94b6452cc6de582584f21066cc5e36d9530c59 (your commit
> 7c6e81b68539422a1dee4f602f6a62ab402bdb01) that has function
> sd_change_bank_voltage.  What's the rational behind this?  Can't we get
> some clean cherry-picks?

No, we can't get clean cherry-picks, this driver uses some very new
features which 3.2 or 3.5 don't have. I signed off if the commit is
changed by me.

It uses mod_delayed_work(), new mmc voltage related feature
(upstream commit 55c4665ea0a42fd6427826bfce96eb4b0389262a). I need to do
some changes, or it won't work. PS, most of these changes comes from the
vendor(Realtek). 

This is the first bug I submitting patches to our kernel tree(I came to
Canonical this month), sorry for the trouble. Do I need modify the
commit messages and submit them again?
Brad Figg - Jan. 29, 2013, 3:59 a.m.
On 01/27/2013 11:03 PM, Adam Lee wrote:
> BugLink: http://launchpad.net/bugs/1057089
> 
> Backported the upstream realtek pcie card reader driver.
> 
> ====
> 
> The following changes since commit 07daba920a41ed25fcfeee6fa7cf29c3073eae57:
> 
>   drivers/mfd: Add realtek pcie card reader driver (2013-01-28 14:07:45 +0800)
> 
> are available in the git repository at:
> 
>   git://kernel.ubuntu.com/adamlee/ubuntu-precise.git master
> 
> for you to fetch changes up to 2fe331e351e465e2b9d262cb5842c350ed139318:
> 
>   [Config] Enable RTSX_PCI modules (2013-01-28 14:07:49 +0800)
> 
> ----------------------------------------------------------------
> Adam Lee (1):
>       [Config] Enable RTSX_PCI modules
> 
> Wei WANG (6):
>       mmc: Add realtek pcie sdmmc host driver
>       mmc: rtsx: Remove a duplicate command in sd_rw_multi
>       mmc: rtsx: Configure SD_CFG2 register in sd_rw_multi
>       mmc: rtsx: Explicitely include slab.h in rtsx_pci_sdmmc.c
>       MFD:rtsx:Avoid kernel panic if rtsx_pci_sdmmc not probed
>       drivers/memstick: Add realtek pcie memstick host driver
> 
>  debian.master/config/config.common.ubuntu |    3 +
>  drivers/memstick/host/Kconfig             |   10 +
>  drivers/memstick/host/Makefile            |    1 +
>  drivers/memstick/host/rtsx_pci_ms.c       |  641 ++++++++++++++
>  drivers/mfd/rtsx_pcr.c                    |    4 +-
>  drivers/mmc/host/Kconfig                  |    7 +
>  drivers/mmc/host/Makefile                 |    2 +
>  drivers/mmc/host/rtsx_pci_sdmmc.c         | 1326 +++++++++++++++++++++++++++++
>  8 files changed, 1992 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/memstick/host/rtsx_pci_ms.c
>  create mode 100644 drivers/mmc/host/rtsx_pci_sdmmc.c
> 

I'm ok with this going into Q, a HWE kernel but I'm not willing
to take it for Precise.

Brad
Brad Figg - Jan. 29, 2013, 4:05 a.m.
On 01/28/2013 07:59 PM, Brad Figg wrote:
> On 01/27/2013 11:03 PM, Adam Lee wrote:
>> BugLink: http://launchpad.net/bugs/1057089
>>
>> Backported the upstream realtek pcie card reader driver.
>>
>> ====
>>
>> The following changes since commit 07daba920a41ed25fcfeee6fa7cf29c3073eae57:
>>
>>   drivers/mfd: Add realtek pcie card reader driver (2013-01-28 14:07:45 +0800)
>>
>> are available in the git repository at:
>>
>>   git://kernel.ubuntu.com/adamlee/ubuntu-precise.git master
>>
>> for you to fetch changes up to 2fe331e351e465e2b9d262cb5842c350ed139318:
>>
>>   [Config] Enable RTSX_PCI modules (2013-01-28 14:07:49 +0800)
>>
>> ----------------------------------------------------------------
>> Adam Lee (1):
>>       [Config] Enable RTSX_PCI modules
>>
>> Wei WANG (6):
>>       mmc: Add realtek pcie sdmmc host driver
>>       mmc: rtsx: Remove a duplicate command in sd_rw_multi
>>       mmc: rtsx: Configure SD_CFG2 register in sd_rw_multi
>>       mmc: rtsx: Explicitely include slab.h in rtsx_pci_sdmmc.c
>>       MFD:rtsx:Avoid kernel panic if rtsx_pci_sdmmc not probed
>>       drivers/memstick: Add realtek pcie memstick host driver
>>
>>  debian.master/config/config.common.ubuntu |    3 +
>>  drivers/memstick/host/Kconfig             |   10 +
>>  drivers/memstick/host/Makefile            |    1 +
>>  drivers/memstick/host/rtsx_pci_ms.c       |  641 ++++++++++++++
>>  drivers/mfd/rtsx_pcr.c                    |    4 +-
>>  drivers/mmc/host/Kconfig                  |    7 +
>>  drivers/mmc/host/Makefile                 |    2 +
>>  drivers/mmc/host/rtsx_pci_sdmmc.c         | 1326 +++++++++++++++++++++++++++++
>>  8 files changed, 1992 insertions(+), 2 deletions(-)
>>  create mode 100644 drivers/memstick/host/rtsx_pci_ms.c
>>  create mode 100644 drivers/mmc/host/rtsx_pci_sdmmc.c
>>
> 
> I'm ok with this going into Q, a HWE kernel but I'm not willing
> to take it for Precise.
> 
> Brad
> 

I should have mentioned that I chatted with HWE before NAKing this. They
are OK with it just going into Q.
Tim Gardner - Jan. 29, 2013, 1:50 p.m.
On 01/28/2013 08:00 PM, Adam Lee wrote:
>
> This is the first bug I submitting patches to our kernel tree(I came to
> Canonical this month), sorry for the trouble. Do I need modify the
> commit messages and submit them again?
>

Yes - you need to resubmit with full attribution, bug links, upstream 
SHA1s, etc, in the commit log. See 
https://wiki.ubuntu.com/Kernel/Dev/KernelPatches

rtg