| Message ID | 1487303780-234812-2-git-send-email-shawn.lin@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 17.2.2017 5:56, Shawn Lin wrote: > We need comment there to tell folks that we need this > callback to return the timeout clock rate in KHz. Good idea, except that currently in some cases it should return the rate in MHz instead so the added comment is actually wrong. Otherwise the patchset seems correct to me, and the timeout clock situation would be better than before. One option would be to just have the callback return Hz as that seems to be the most obvious unit. The callback would probably have to be renamed for that, though, to avoid any backporting etc. headaches later. > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> > --- > > drivers/mmc/host/sdhci.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h > index edf3adf..d5b4d57 100644 > --- a/drivers/mmc/host/sdhci.h > +++ b/drivers/mmc/host/sdhci.h > @@ -547,6 +547,7 @@ struct sdhci_ops { > int (*enable_dma)(struct sdhci_host *host); > unsigned int (*get_max_clock)(struct sdhci_host *host); > unsigned int (*get_min_clock)(struct sdhci_host *host); > + /* get_timeout_clock should return clk rate in KHz */ > unsigned int (*get_timeout_clock)(struct sdhci_host *host); > unsigned int (*get_max_timeout_count)(struct sdhci_host *host); > void (*set_timeout)(struct sdhci_host *host,
Hi Anssi, On 2017/2/24 22:00, Anssi Hannula wrote: > On 17.2.2017 5:56, Shawn Lin wrote: >> We need comment there to tell folks that we need this >> callback to return the timeout clock rate in KHz. > > Good idea, except that currently in some cases it should return the rate > in MHz instead so the added comment is actually wrong. > Ahh... you are right. It depends on the timeout clock unit of capability register... > Otherwise the patchset seems correct to me, and the timeout clock > situation would be better than before. > > > One option would be to just have the callback return Hz as that seems to > be the most obvious unit. The callback would probably have to be renamed > for that, though, to avoid any backporting etc. headaches later. Makes sense to me as I think most modem sdhci variant controllers get these clock from the common clock framework which provide the clk rate in Hz unit. > > >> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> >> --- >> >> drivers/mmc/host/sdhci.h | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h >> index edf3adf..d5b4d57 100644 >> --- a/drivers/mmc/host/sdhci.h >> +++ b/drivers/mmc/host/sdhci.h >> @@ -547,6 +547,7 @@ struct sdhci_ops { >> int (*enable_dma)(struct sdhci_host *host); >> unsigned int (*get_max_clock)(struct sdhci_host *host); >> unsigned int (*get_min_clock)(struct sdhci_host *host); >> + /* get_timeout_clock should return clk rate in KHz */ >> unsigned int (*get_timeout_clock)(struct sdhci_host *host); >> unsigned int (*get_max_timeout_count)(struct sdhci_host *host); >> void (*set_timeout)(struct sdhci_host *host, > >
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h index edf3adf..d5b4d57 100644 --- a/drivers/mmc/host/sdhci.h +++ b/drivers/mmc/host/sdhci.h @@ -547,6 +547,7 @@ struct sdhci_ops { int (*enable_dma)(struct sdhci_host *host); unsigned int (*get_max_clock)(struct sdhci_host *host); unsigned int (*get_min_clock)(struct sdhci_host *host); + /* get_timeout_clock should return clk rate in KHz */ unsigned int (*get_timeout_clock)(struct sdhci_host *host); unsigned int (*get_max_timeout_count)(struct sdhci_host *host); void (*set_timeout)(struct sdhci_host *host,
We need comment there to tell folks that we need this callback to return the timeout clock rate in KHz. Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> --- drivers/mmc/host/sdhci.h | 1 + 1 file changed, 1 insertion(+)