Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 7 Aug 2023 13:59:23 +0300
From:   Tony Lindgren <tony@atomide.com>
To:     Dhruva Gole <d-gole@ti.com>
Cc:     Nishanth Menon <nm@ti.com>,
        Linus Walleij <linus.walleij@linaro.org>,
        linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-omap@vger.kernel.org, linux-gpio@vger.kernel.org,
        Vignesh Raghavendra <vigneshr@ti.com>
Subject: Re: [PATCH] pinctrl: single: Add compatible for ti,am625-padconf
Message-ID: <20230807105923.GO14799@atomide.com>
References: <20230805045554.786092-1-d-gole@ti.com>
 <20230805171508.schg4xquoa24klk5@october>
 <20230807070724.GN14799@atomide.com>
 <20230807080922.t35yvyvrknjcriyr@dhruva>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230807080922.t35yvyvrknjcriyr@dhruva>
Precedence: bulk

* Dhruva Gole <d-gole@ti.com> [230807 08:09]:
> On Aug 07, 2023 at 10:07:24 +0300, Tony Lindgren wrote:
> > * Nishanth Menon <nm@ti.com> [230805 17:15]:
> > > On 10:25-20230805, Dhruva Gole wrote:
> > > > From: Tony Lindgren <tony@atomide.com>
> > > > +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?

Yes, but SoC specific defines needs to start with a SoC specific
prefix as multiple files may be included for various SoCs.

> 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 wake_en only gets set when some driver does request_irq() on
the wakeirq. No need to set them all.

> Sure, I could take a look, but setting wake_en on all pads still
> doesn't feel right to me.

No need to set all wake_en pads, just checking that if request_irq()
is done for some pin that does not have wake_en capability does not
cause eternal interrupts if a reserved bit is high all the time :)

Regards,

Tony