On Aug 07, 2023 at 10:07:24 +0300, Tony Lindgren wrote:
> * Nishanth Menon <nm@xxxxxx> [230805 17:15]:
> > On 10:25-20230805, Dhruva Gole wrote:
> > > From: Tony Lindgren <tony@xxxxxxxxxxx>
> > > +static const struct pcs_soc_data pinctrl_single_am625 = {
> > > + .flags = PCS_QUIRK_SHARED_IRQ | PCS_CONTEXT_LOSS_OFF,
> > > + .irq_enable_mask = (1 << 29), /* WKUP_EN */
> > > + .irq_status_mask = (1 << 30), /* WKUP_EVT */
> > > +};
> > > +
> >
> > Why cant we set this in the k3-pinctrl.h and set it once?
Do you mean that I set 1 << 29 and 30 as sort of macros in the
k3-pinctrl.h file and then include it in pinctrl-single.c?
Are we okay to #include a header from arch/arm64/boot/dts/ti?
>
> Good idea to define the bit offsets k3-pinctrl.h instead of magic numbers
> here :)
If I understand what Nishanth is saying correctly, are we expected to
set the wake_en bit on every single K3 SoC's every single padconf reg?
I am a little sceptical with this approach, because what is people
_don't_ want to wakeup from certain pads? What would be the right way to
disable wakeup on those pads then?
>
> > The event will not be generated until wakeup daisy chain is triggered
> > anyways.
Any voltage level shift can potentially trigger a daisychain and I don't
think that's really such a good idea?
>
> Yup, and having that happen is enough to show the wake-up reason with
> grep wakeup /proc/interrupts :)
>
> > Have you looked at all the padconf registers across devices to ensure
> > the WKUP_EN/EVT bits are present? daisy chain feature is used elsewhere
> > as well.
In my limited experience, I have only seen daisychain wakeups being
enabled on AM62x SOC. This is because this is one of the first K3
devices to implement deepsleep, and I think IO daisychain only applies for
wakeups in the case of deepsleep kind of scenarios.
>
> The lack of bits at least earlier just meant that attempting to use a
> wake-up interrupt would just never trigger. Worth checking though.
> Dhruva, care to check if some padconf register have reserved bits for
> 29 and 30 that might be set high by default?
Sure, I could take a look, but setting wake_en on all pads still
doesn't feel right to me.
>
> Regards,
>
> Tony
To summarise, I don't think any other devices are using daisychain
atleast today, and even if there is possibility of using in future I
think the same compatible I have used here can be used to set wake_en
wherever applicable, for eg. whenever AM62A would want to use daisychain
it can use this quirk in it's DT node.
I believe that we shouldn't set every pad as daisychain enabled
otherwise in deepsleep it may result in unintended wakeups. And the way
I thought we can give this choice to the user is using wakeirq chained
interrupt along with this quirk,
compatible = "ti,am6-padconf";
--
Best regards,
Dhruva Gole <d-gole@xxxxxx>
