Re: [PATCH v2 19/48] opp: Fix adding OPP entries in a wrong order if rate is unavailable
24.12.2020 09:28, Viresh Kumar пишет:
> On 23-12-20, 23:36, Dmitry Osipenko wrote:
>> 23.12.2020 07:34, Viresh Kumar пишет:
>>> On 22-12-20, 22:19, Dmitry Osipenko wrote:
22.12.2020 12:12, Viresh Kumar пишет:
> rate will be 0 for both the OPPs here if rate_not_available is true and
> so this
> change shouldn't be required.
The rate_not_available is negated in the condition. This change is
required because both rates are 0 and then we should proceed to the
levels comparison.
>>>
>>> Won't that happen without this patch ?
>>
>> No
>
> This is how the code looks like currently:
>
> int _opp_compare_key(struct dev_pm_opp *opp1, struct dev_pm_opp *opp2)
> {
> if (opp1->rate != opp2->rate)
> return opp1->rate < opp2->rate ? -1 : 1;
> if (opp1->bandwidth && opp2->bandwidth &&
> opp1->bandwidth[0].peak != opp2->bandwidth[0].peak)
> return opp1->bandwidth[0].peak < opp2->bandwidth[0].peak ? -1 :
> 1;
> if (opp1->level != opp2->level)
> return opp1->level < opp2->level ? -1 : 1;
> return 0;
> }
>
> Lets consider the case you are focussing on, where rate is 0 for both the
> OPPs,
> bandwidth isn't there and we want to run the level comparison here.
>
> Since both the rates are 0, (opp1->rate != opp2->rate) will fail and so we
> will
> move to bandwidth check which will fail too. And so we will get to the level
> comparison.
>
> What am I missing here ? I am sure there is something for sure as you won't
> have
> missed this..
>
Ah, you're right. It was me who was missing something as I see now,
after taking a closer look and trying to implement yours suggestion, my
bad. I'll improve this patch in the next revision, thanks!
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 28/48] soc/tegra: Introduce core power domain driver
24.12.2020 09:51, Viresh Kumar пишет: > On 23-12-20, 23:37, Dmitry Osipenko wrote: >> 23.12.2020 08:57, Viresh Kumar пишет: >>> What's wrong with getting the regulator in the driver as well ? Apart from >>> the >>> OPP core ? >> >> The voltage syncing should be done for each consumer regulator >> individually [1]. >> >> Secondly, regulator core doesn't work well today if the same regulator >> is requested more than one time for the same device. > > Hmm... > will return the OPP table regulator in order to allow driver to use the regulator directly. But I'm not sure whether this is a much better option than the opp_sync_regulators() and opp_set_voltage() APIs. >>> >>> set_voltage() is still fine as there is some data that the OPP core has, but >>> sync_regulator() has nothing to do with OPP core. >>> >>> And this may lead to more wrapper helpers in the OPP core, which I am >>> afraid of. >>> And so even if it is not the best, I would like the OPP core to provide the >>> data >>> and not get into this. Ofcourse there is an exception to this, opp_set_rate. >>> >> >> The regulator_sync_voltage() should be invoked only if voltage was >> changed previously [1]. >> >> The OPP core already has the info about whether voltage was changed and >> it provides the necessary locking for both set_voltage() and >> sync_regulator(). Perhaps I'll need to duplicate that functionality in >> the PD driver, instead of making it all generic and re-usable by other >> drivers. >> >> [1] >> https://elixir.bootlin.com/linux/v5.10.2/source/drivers/regulator/core.c#L4107 > > Lets do it in the OPP core and see where we go. > Alright, thank you. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 15/48] opp: Support set_opp() customization without requiring to use regulators
24.12.2020 07:10, Viresh Kumar пишет: > On 23-12-20, 23:38, Dmitry Osipenko wrote: >> Well, there is no "same structure", the opp_table->set_opp_data is NULL >> there. > > Right, I saw that yesterday. What I meant was that we need to start allocating > the structure for this case now. > Okay, that will be a bit bigger change than this v2. I'll try it implement yours suggestion in the next revision, thanks. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 11/48] opp: Add dev_pm_opp_find_level_ceil()
24.12.2020 09:43, Viresh Kumar пишет: > On 23-12-20, 23:37, Dmitry Osipenko wrote: >> 23.12.2020 07:19, Viresh Kumar пишет: >>> On 22-12-20, 22:15, Dmitry Osipenko wrote: 22.12.2020 09:42, Viresh Kumar пишет: > On 17-12-20, 21:06, Dmitry Osipenko wrote: >> Add a ceil version of the dev_pm_opp_find_level(). It's handy to have if >> levels don't start from 0 in OPP table and zero usually means a minimal >> level. >> >> Signed-off-by: Dmitry Osipenko > > Why doesn't the exact version work for you here ? > The exact version won't find OPP for level=0 if levels don't start with 0, where 0 means that minimal level is desired. >>> >>> Right, but why do you need to send 0 for your platform ? >>> >> >> To put power domain into the lowest performance state when device is idling. > > I see. So you really want to set it to the lowest state or just take the vote > out ? Which may end up powering off the domain in the worst case ? > In a device driver I want to set PD to the lowest performance state by removing the performance vote when dev_pm_opp_set_rate(dev, 0) is invoked by the driver. The OPP core already does this, but if OPP levels don't start from 0 in a device-tree for PD, then it currently doesn't work since there is a need to get a rounded-up performance state because dev_pm_opp_set_voltage() takes OPP entry for the argument (patches 9 and 28). The PD powering off and performance-changes are separate from each other in the GENPD core. The GENPD core automatically turns off domain when all devices within the domain are suspended by system-suspend or RPM. The performance state of a power domain is controlled solely by a device driver. GENPD core only aggregates the performance requests, it doesn't change the performance state of a domain by itself when device is suspended or resumed, IIUC this is intentional. And I want to put domain into lowest performance state when device is suspended. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 -next] staging: most: net: use DEFINE_MUTEX() for mutex lock
mutex lock can be initialized automatically with DEFINE_MUTEX()
rather than explicitly calling mutex_init().
Signed-off-by: Zheng Yongjun
---
drivers/staging/most/net/net.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/most/net/net.c b/drivers/staging/most/net/net.c
index b6fecb06a0e6..f125bb6da406 100644
--- a/drivers/staging/most/net/net.c
+++ b/drivers/staging/most/net/net.c
@@ -68,7 +68,7 @@ struct net_dev_context {
};
static struct list_head net_devices = LIST_HEAD_INIT(net_devices);
-static struct mutex probe_disc_mt; /* ch->linked = true, most_nd_open */
+static DEFINE_MUTEX(probe_disc_mt); /* ch->linked = true, most_nd_open */
static DEFINE_SPINLOCK(list_lock); /* list_head, ch->linked = false, dev_hold
*/
static struct most_component comp;
@@ -520,7 +520,6 @@ static int __init most_net_init(void)
{
int err;
- mutex_init(&probe_disc_mt);
err = most_register_component(&comp);
if (err)
return err;
--
2.22.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 -next] staging: vc04_services: use DEFINE_MUTEX() for mutex lock
mutex lock can be initialized automatically with DEFINE_MUTEX()
rather than explicitly calling mutex_init().
Signed-off-by: Zheng Yongjun
---
.../vc04_services/interface/vchiq_arm/vchiq_connected.c| 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git
a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
index 79b75efa6868..864253866155 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
@@ -12,13 +12,12 @@ static intg_connected;
static intg_num_deferred_callbacks;
static VCHIQ_CONNECTED_CALLBACK_T g_deferred_callback[MAX_CALLBACKS];
static intg_once_init;
-static struct mutex g_connected_mutex;
+static DEFINE_MUTEX(g_connected_mutex);
/* Function to initialize our lock */
static void connected_init(void)
{
if (!g_once_init) {
- mutex_init(&g_connected_mutex);
g_once_init = 1;
}
}
--
2.22.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723bs: clean up brace coding style issues
On Tue, 2020-12-22 at 15:17 +0100, Michael Straube wrote:
> Add missing braces around else arm of if else statement to clear
> style issues reported by checkpatch.
>
> CHECK: braces {} should be used on all arms of this statement
> CHECK: Unbalanced braces around else statement
[]
> diff --git a/drivers/staging/rtl8723bs/core/rtw_efuse.c
> b/drivers/staging/rtl8723bs/core/rtw_efuse.c
[]
> @@ -245,8 +245,9 @@ u16 Address)
> break;
> }
> return rtw_read8(Adapter, EFUSE_CTRL);
> - } else
> + } else {
> return 0xFF;
> + }
Instead, when you see a pattern like this it's generally
better to reverse whatever test is above this, return early
and unindent the block above the else.
Here that would be:
---
diff --git a/drivers/staging/rtl8723bs/core/rtw_efuse.c
b/drivers/staging/rtl8723bs/core/rtw_efuse.c
index 32ca10f01413..e5c3dba5c8ae 100644
--- a/drivers/staging/rtl8723bs/core/rtw_efuse.c
+++ b/drivers/staging/rtl8723bs/core/rtw_efuse.c
@@ -222,31 +222,31 @@ u16 Address)
EFUSE_GetEfuseDefinition(Adapter, EFUSE_WIFI,
TYPE_EFUSE_REAL_CONTENT_LEN, (void *)&contentLen, false);
- if (Address < contentLen) {/* E-fuse 512Byte */
- /* Write E-fuse Register address bit0~7 */
- temp = Address & 0xFF;
- rtw_write8(Adapter, EFUSE_CTRL+1, temp);
- Bytetemp = rtw_read8(Adapter, EFUSE_CTRL+2);
- /* Write E-fuse Register address bit8~9 */
- temp = ((Address >> 8) & 0x03) | (Bytetemp & 0xFC);
- rtw_write8(Adapter, EFUSE_CTRL+2, temp);
-
- /* Write 0x30[31]= 0 */
- Bytetemp = rtw_read8(Adapter, EFUSE_CTRL+3);
- temp = Bytetemp & 0x7F;
- rtw_write8(Adapter, EFUSE_CTRL+3, temp);
+ if (Address >= contentLen) /* E-fuse 512Byte */
+ return 0xFF;
- /* Wait Write-ready (0x30[31]= 1) */
+ /* Write E-fuse Register address bit0~7 */
+ temp = Address & 0xFF;
+ rtw_write8(Adapter, EFUSE_CTRL+1, temp);
+ Bytetemp = rtw_read8(Adapter, EFUSE_CTRL+2);
+ /* Write E-fuse Register address bit8~9 */
+ temp = ((Address >> 8) & 0x03) | (Bytetemp & 0xFC);
+ rtw_write8(Adapter, EFUSE_CTRL+2, temp);
+
+ /* Write 0x30[31]= 0 */
+ Bytetemp = rtw_read8(Adapter, EFUSE_CTRL+3);
+ temp = Bytetemp & 0x7F;
+ rtw_write8(Adapter, EFUSE_CTRL+3, temp);
+
+ /* Wait Write-ready (0x30[31]= 1) */
+ Bytetemp = rtw_read8(Adapter, EFUSE_CTRL+3);
+ while (!(Bytetemp & 0x80)) {
Bytetemp = rtw_read8(Adapter, EFUSE_CTRL+3);
- while (!(Bytetemp & 0x80)) {
- Bytetemp = rtw_read8(Adapter, EFUSE_CTRL+3);
- k++;
- if (k == 1000)
- break;
- }
- return rtw_read8(Adapter, EFUSE_CTRL);
- } else
- return 0xFF;
+ k++;
+ if (k == 1000)
+ break;
+ }
+ return rtw_read8(Adapter, EFUSE_CTRL);
} /* EFUSE_Read1Byte */
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: ralink-gdma: Fixed blank line coding style issue
On Wed, 2020-12-23 at 21:22 +0100, Ayoub Soussi wrote:
> Fixed coding style issue.
[]
> diff --git a/drivers/staging/ralink-gdma/ralink-gdma.c
> b/drivers/staging/ralink-gdma/ralink-gdma.c
[]
> @@ -122,6 +122,7 @@ struct gdma_dma_dev {
> struct gdma_data *data;
> void __iomem *base;
> struct tasklet_struct task;
> +
> volatile unsigned long chan_issued;
> atomic_t cnt;
This is presumably a checkpatch false positive.
checkpatch is not now nor never will be a perfect tool.
Please consider what you are doing and what the desired coding style is
before submitting patches.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
