On 12/10/24 16:17, Mukesh Kumar Savaliya wrote:
Thanks Konrad !
On 12/10/2024 6:08 PM, Konrad Dybcio wrote:
On 12/10/24 13:05, Krzysztof Kozlowski wrote:
On 10/12/2024 12:53, Krzysztof Kozlowski wrote:
I'm not sure a single property name+description can fit all possible
cases here. The hardware being "shared" can mean a number of different
Existing property does not explain anything more, either. To recap -
this block is SE and property is named "se-shared", so basically it is
equal to just "shared". "shared" is indeed quite vague, so I was
expecting some wider work here.
things, with some blocks having hardware provisions for that, while
others may have totally none and rely on external mechanisms (e.g.
a shared memory buffer) to indicate whether an external entity
manages power to them.
We have properties for that too. Qualcomm SoCs need once per year for
such shared properties. BAM has two or three. IPA has two. There are
probably even more blocks which I don't remember now.
So, the problem is "driver must not toggle GPIO states", because
"the bus controller must not be muxed away from the endpoint".
You can come up with a number of similar problems by swapping out
the quoted text.
We can either describe what the driver must do (A), or what the
reason for it is (B).
If we go with A, we could have a property like:
&i2c1 {
externally-handled-resources = <(EHR_PINCTRL_STATE | EHR_CLOCK_RATE)>
};
which would be a generic list of things that the OS would have to
tiptoe around, fitting Linux's framework split quite well
or if we go with B, we could add a property like:
&i2c1 {
qcom,shared-controller;
};
which would hide the implementation details into the driver
I could see both approaches having their place, but in this specific
instance I think A would be more fitting, as the problem is quite
simple.
The second is fine with me, maybe missing information about "whom" do
you share it with. Or maybe we get to the point that all this is
specific to SoC, thus implied by compatible and we do not need
downstream approach (another discussion in USB pushed by Qcom: I want
one compatible and 1000 properties).
I really wished Qualcomm start reworking their bindings before they are
being sent upstream to match standard DT guidelines, not downstream
approach. Somehow these hundreds reviews we give could result in new
patches doing things better, not just repeating the same issues.
This is BTW v5, with all the same concerns from v1 and still no answers
in commit msg about these concerns. Nothing explained in commit msg
which hardware needs it or why the same SoC have it once shared, once
not (exclusive). Basically there is nothing here corresponding to any
real product, so since five versions all this for me is just copy-paste
from downstream approach.
So since this is a software contract and not a hardware
feature, this is not bound to any specific SoC or "firmware",
but rather to what runs on other cores (e.g. DSPs, MCUs spread
across the SoC or in a different software world, like TZ).
Specifying the specific intended use would be helpful though,
indeed.
Let's see if we can somehow make this saner.
Mukesh, do we have any spare registers that we could use to
indicate that a given SE is shared? Preferably within the
SE's register space itself. The bootloader or another entity
(DSP or what have you) would then set that bit before Linux
runs and we could skip the bindings story altogether.
There would be spare register but i think it should be in sync with hardware team. let me check with them and update back if any bit can be repurposed for this feature. I agree, if any register is available, it can programmed prior to kernel.
It would need to be reserved on all SoCs though (future and
past), to make sure the contract is always held up, but I
think finding a persistent bit that has never been used
shouldn't be impossible.
Yes, let me check it with hardware and firmware team and update back. Does this mean, there can't be a such software sharing mechanism (purely software decision) based on DTSI flag ?
I suppose that depends on our needs. If we can set that bit
before Linux starts (i.e. in UEFI), we can avoid touching
the pinctrl state regardless of whether the other entities
have started up yet to avoid overcomplicating it.
If we need Linux to set that bit, we would still need some
mechanism like a dt property. But I really think that the
bootloader should be burdened with this instead, given it
has a better understanding of the hardware due to it being
well, the bootloader).
Krzysztof, I'm assuming that sounds sane from your
perspective too?
Konrad
