linux-kernel - Re: [PATCH 2/6] clk: qcom: gdsc: Add support for gdscs with gds hw controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <fa3b0189ad586a846f398d3189505dca.squirrel@www.codeaurora.org>
Date:	Tue, 1 Dec 2015 16:01:42 -0000
From:	"Rajendra Nayak" <rnayak@...eaurora.org>
To:	"Stephen Boyd" <sboyd@...eaurora.org>
Cc:	"Rajendra Nayak" <rnayak@...eaurora.org>, mturquette@...libre.com,
	linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org,
	linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH 2/6] clk: qcom: gdsc: Add support for gdscs with gds hw
 controller

[]..

>> >>  	return -ETIMEDOUT;
>> >>  }
>> >>
>> >> @@ -165,6 +169,7 @@ static int gdsc_init(struct gdsc *sc)
>> >>  {
>> >>  	u32 mask, val;
>> >>  	int on, ret;
>> >> +	unsigned int reg;
>> >>
>> >>  	/*
>> >>  	 * Disable HW trigger: collapse/restore occur based on registers
>> writes.
>> >> @@ -185,7 +190,8 @@ static int gdsc_init(struct gdsc *sc)
>> >>  			return ret;
>> >>  	}
>> >>
>> >> -	on = gdsc_is_enabled(sc);
>> >> +	reg = sc->gds_hw_ctrl ? sc->gds_hw_ctrl : sc->gdscr;
>> >> +	on = gdsc_is_enabled(sc, reg);
>> >
>> > If the gdsc is voteable, then we need to make sure that the vote
>> > is from us when we boot up. Otherwise the kernel may think that
>> > the gdsc is enabled, but it gets turned off by some other master
>> > later on. I don't know if this causes some sort of problem for
>> > the power domain framework, but we can't rely on the status bit
>> > unless we're sure that we've actually set the register to enable
>> > it. In the normal enable/disable path we'll always know we set
>> > the register, so this really only matters once when we boot up.
>>
>> right, thanks for catching this. However if we vote for a votable
>> GDSC just because its ON at boot (due to someone else having voted)
>> we won't ever remove the vote keeping it always enabled.
>>
>> I think a safe way would be to consider all votable gdscs for which
>> *we* haven't voted explicitly to be disabled at boot?
>>
>
> Agreed, when we boot we should consider GDSCs that are indicating
> they're enabled via the bit 31 status bit but without the sw
> enable mask set as "disabled" even though they're actually
> enabled by some other master in the SoC.

Thinking about this a bit more, your earlier suggestion of voting
for the GDSC explicitly seemed to work too, and also seemed cleaner.
genpd ends up removing the vote if there aren't any users as part
of genpd_poweroff_unused()

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/