Messages in this thread | | | 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 06:51:39 +0100 |
| |
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.
Chris
| |