| Message ID | 20130128070321.GC10628@adam-laptop |
|---|---|
| State | New |
| Headers | show |
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
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?
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
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.
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
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