Messages in this thread Patch in this message | | | Subject | Re: PATCH: rtsx_pci driver - don't disable the rts5229 card reader on Intel NUC boxes | From | Chris Clayton <> | Date | Wed, 5 Aug 2020 13:48:23 +0100 |
| |
Hi Ricky
On 05/08/2020 06:51, Chris Clayton wrote: > Thanks, Ricky. > > On 05/08/2020 03:35, 吳昊澄 Ricky wrote: >> >> >>> -----Original Message----- >>> From: Chris Clayton [mailto:chris2553@googlemail.com] >>> Sent: Tuesday, August 04, 2020 7:52 PM >>> To: 吳昊澄 Ricky; gregkh@linuxfoundation.org >>> Cc: LKML; rdunlap@infradead.org; philquadra@gmail.com; Arnd Bergmann >>> Subject: Re: PATCH: rtsx_pci driver - don't disable the rts5229 card reader on >>> Intel NUC boxes >>> >>> >>> >>> On 04/08/2020 11:46, 吳昊澄 Ricky wrote: >>>>> -----Original Message----- >>>>> From: Chris Clayton [mailto:chris2553@googlemail.com] >>>>> Sent: Tuesday, August 04, 2020 4:51 PM >>>>> To: 吳昊澄 Ricky; gregkh@linuxfoundation.org >>>>> Cc: LKML; rdunlap@infradead.org; philquadra@gmail.com; Arnd Bergmann >>>>> Subject: Re: PATCH: rtsx_pci driver - don't disable the rts5229 card reader on >>>>> Intel NUC boxes >>>>> >>>>> >>>>> >>>>> On 04/08/2020 09:08, 吳昊澄 Ricky wrote: >>>>>>> -----Original Message----- >>>>>>> From: gregkh@linuxfoundation.org [mailto:gregkh@linuxfoundation.org] >>>>>>> Sent: Tuesday, August 04, 2020 3:49 PM >>>>>>> To: 吳昊澄 Ricky >>>>>>> Cc: Chris Clayton; LKML; rdunlap@infradead.org; philquadra@gmail.com; >>>>> Arnd >>>>>>> Bergmann >>>>>>> Subject: Re: PATCH: rtsx_pci driver - don't disable the rts5229 card reader >>> on >>>>>>> Intel NUC boxes >>>>>>> >>>>>>> On Tue, Aug 04, 2020 at 02:44:41AM +0000, 吳昊澄 Ricky wrote: >>>>>>>> Hi Chris, >>>>>>>> >>>>>>>> rtsx_pci_write_register(pcr, FPDTL, OC_POWER_DOWN, >>>>> OC_POWER_DOWN); >>>>>>>> This register operation saved power under 1mA, so if do not care the 1mA >>>>>>> power we can have a patch to remove it, make compatible with NUC6 >>>>>>>> We tested others our card reader that remove it, we did not see any side >>>>> effect >>>>>>>> >>>>>>>> Hi Greg k-h, >>>>>>>> >>>>>>>> Do you have any comments? >>>>>>> >>>>>>> comments on what? I don't know what you are responding to here, sorry. >>>>>>> >>>>>> Can we have a patch to kernel for NUC6? It may cause more power(1mA) but >>> it >>>>> will have more compatibility >>>>>> >>>>> >>>>> Ricky, >>>>> >>>>> I don't understand why you want to completely remove the code that sets up >>> the >>>>> 1mA power saving. That code was there >>>>> even before your patch (bede03a579b3b4a036003c4862cc1baa4ddc351f), so I >>>>> assume it benefits some of the Realtek card >>>>> readers. Before your patch however, rtsx_pci_init_ocp() was not called for the >>>>> rts5229 reader, but the patch introduced >>>>> an unconditional call to that function into rtsx_pci_init_hw(), which is run for >>> the >>>>> rts5229. That is what now disables >>>>> the card reader. >>>>> >>>>> Now, I don't know whether other cards are affected, although I don't recall >>>>> seeing any reported as I searched the kernel >>>>> and ubuntu bugzillas for any analysis of the problem. I know this is not what >>> the >>>>> patch I sent does, but having thought >>>>> about it more, seems to me that the simplest fix is to skip the new call to >>>>> rtsx_pci_init_ocp() if the reader is an rts5229. >>>>> >>>> >>>> Because we are thinking about if others our card reader that not belong A >>> series(my ocp patch coverage) also on NUC6 platform maybe have the same >>> problem... >>>> >>> >>> OK. What if we do make the new call but only for the card readers that are in the >>> A series? Are they the ones that have >>> PID_5nnn defines in include/linux/rtsx_pci.h? Or is there another simple way of >>> identifying that a reader is a member of >>> the A series? >>> >>> I'm thinking of something like: >>> static bool rtsx_pci_is_series_A(pcr) >>> { >>> unsigned short device = pcr->pci->device; >>> >>> return device == PID524A || device == PID_5249 || device == PID_5250 || >>> device == PID_525A >>> || device == PID_525A || device == PID_5260 || device == >>> PID_5261; >>> } >>> >>> then in rtsx_pci_init_hw() change the unconditional call to: >>> >>> if rtsx_pci_is_series_A(pcr) >>> rtsx_pci_init_ocp(); >>> >>> Does that seem OK? >>> >> Previously, I want to remove >> else { >> /* OC power down */ >> rtsx_pci_write_register(pcr, FPDCTL, OC_POWER_DOWN, >> OC_POWER_DOWN); >> } >> Because in our A-series card Reader we already assigned option->ocp_en to 1 in self init_params() , this is an easy way to fix this problem >> > > Ah, OK. I'll prepare the patch and send it to you once I've tested it. >
Please see the patch included below. As you suggested, it removes the code that does the OC power down on card readers that are not members of your A series. The patch is against a fresh pull of Linus's tree this morning (v5.8-2768-g4da9f3302615).
I've tested the resultant kernel on my Intel NUC6CAYH box (which contains an NUC66AYB board) and the rts5229 works fine. I've also tested it on my laptop which also has a card reader supported by the rtsx_pci driver (an RTL8411B) and that also works fine.
As I mentioned yesterday, I think it's a candidate for the 5.4 ans 5.7 stable series.
Thanks for your patience!
Chris
Signed-off-by: Chris Clayton <chris2553@googlemail.com>
--- a/drivers/misc/cardreader/rtsx_pcr.c 2020-08-05 07:10:21.752072515 +0100 +++ b/drivers/misc/cardreader/rtsx_pcr.c 2020-08-05 07:11:05.568074278 +0100 @@ -1172,10 +1172,6 @@ void rtsx_pci_init_ocp(struct rtsx_pcr * rtsx_pci_write_register(pcr, REG_OCPGLITCH, SD_OCP_GLITCH_MASK, pcr->hw_param.ocp_glitch); rtsx_pci_enable_ocp(pcr); - } else { - /* OC power down */ - rtsx_pci_write_register(pcr, FPDCTL, OC_POWER_DOWN, - OC_POWER_DOWN); } } }
| |