lkml.org 
[lkml]   [2020]   [Sep]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3] misc: rtsx: add power saving function and bios guide options
> > > From: Ricky Wu <ricky_wu@realtek.com>
> > >
> > > Added rts5227 rts5249 rts5260 rts5228 power saving functions,
> > > added BIOS guide MMC funciton and U_d3_en register support and
> > > fixed rts5260 driving parameter
> >
> > This should be split into small logical pieces. I can't really tell
> > what those would be, but just based on the commit message, it could
> > be:
> >
> > 1) Add rts5227 rts5249 rts5260 rts5228 power saving functions
> >
> > 2) Add BIOS guide MMC function and U_d3_en register support
> >
> > 3) Fix rts5260 driving parameter
> >
> > s/funciton/function/
> >
> > It looks like 1) *might* be just this:
> >
> > rts*_force_power_down()
> > {
> > ...
> > rtsx_pci_write_register(pcr, pcr->reg_pm_ctrl3, D3_DELINK_MODE_EN,
> > D3_DELINK_MODE_EN);
> >
> > That should be a single patch by itself so it's obvious that it's
> > doing the same thing to several drivers.
>
> Ok, I will have a extra patch for all xx_force_power_down()

Great, thanks!

> > Explain what "BIOS guide MMC function" means. Mention the name of the
> > function this adds so we can connect the patch with the commit log.
> >
>
> "BIOS guide MMC function" means, via BIOS setting to know MMC card
> support or not

It will be helpful if this is in a patch by itself.

As far as I know, there's no actual BIOS *call* here, so you must be
looking at some setting in the *device* itself, on the assumption that
it was done by the BIOS?

That sounds like it could become a problem if the device is ever reset
or put in a low-power state. For resets, and possibly even a
low-power state, BIOS won't be involved, so the setting will be lost,
and the device may work differently after the reset than it did
before. That sounds undesirable.

> > Explain what "U_d3_en" is; that doesn't even appear in the patch.
>
> I am going to remove U_d3_en from patch description, this mean
> D3_DELINK_MODE_EN register

OK. Given the size of the patch, I think the commit log is too short
to describe what's going on. More details would be helpful.

Bjorn

\
 
 \ /
  Last update: 2020-09-02 14:30    [W:0.133 / U:0.072 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site