Re: [PATCH 9/9] arm64: dts: renesas: rzg3s-smarc-som: Enable I3C

[Wolfram Sang <wsa+renesas@sang-engineering.com>]

[-- Attachment #1: Type: text/plain, Size: 2445 bytes --]

Hi Claudiu,

> Could you please let me know what do you consider we should do here? Do you
> think we could drop these pin controller setting and do some particular I3C
> controller settings instead?

My original thought was: If it was a boolean state which is active when
suspending and disabled when resuming, then we wouldn't need a customer
specific binding for it and just do this in the suspend/resume functions
of the pin-controller...

... BUT ...

reading more about this in the manual, just raises more questions for
me.

> The output is fixed at Hi-Z and no data is transmitted to the inside even if
> data is input from outside. “Standby mode” is available when using I2C mode
> only. (Not available when using I3C mode).
> 
> The current driver proposal don't take into account the IP mode when setting
> STBN though
> pinctrl_pm_select_sleep_state()/pinctrl_pm_select_default_state() to keep
> the code simpler, relying on the "Not available when using I3C mode" part of
> the note, and considering setting it when the IP is in I3C mode is harmless.

This is one question I also had: What does "not available" actually
mean? Did you confirm with HW guys that it is really harmles?

I also wonder about the intended use-case of this mode. "no data is
transmitted to the inside even if data is input from outside" doesn't
really sound like a mode intended when the whole SoC goes to sleep. Why
or how would input be even transmitted to the inside if everything is in
a deep-sleep state? I could also imagine that this mode is rather used
to hide from the bus for a while for some corner-case reason.

And finally: does this really save energy? Could you measure a benefit?
Maybe there is nothing driven at all in the sleep state? Then, nothing
is gained? Not clear from the datasheet.

Because the datasheet is so sparse with information and because it
doesn't say how STBN is intended to be used, I would argue we should
skip it until we know what it is for and how it is used. If we know this
somewhen, we can still add this in a second step.

But for now, enabling I3C realiably is the first step, and for that we
surley need the POC bit to select the voltage. This is easily
understandable and straightforward to do. So, my suggestion is to pick
this low-hanging fruit now and reach for the other one once we have more
information about it.

Happy hacking,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]