Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Thu, 21 Apr 2022 09:51:10 +0300
From:   Tony Lindgren <tony@atomide.com>
To:     Puranjay Mohan <p-mohan@ti.com>
Cc:     linux-kernel@vger.kernel.org, nm@ti.com,
        devicetree@vger.kernel.org, grygorii.strashko@ti.com,
        vigneshr@ti.com, mathieu.poirier@linaro.org, kishon@ti.com,
        linux-remoteproc@vger.kernel.org, bjorn.andersson@linaro.org,
        rogerq@kernel.org, krzysztof.kozlowski+dt@linaro.org,
        ssantosh@kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 5/6] soc: ti: pruss: Add helper function to enable OCP
 master ports
Message-ID: <YmD+3svXUIHiX6DJ@atomide.com>
References: <20220418123004.9332-1-p-mohan@ti.com>
 <20220418123004.9332-6-p-mohan@ti.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220418123004.9332-6-p-mohan@ti.com>
Precedence: bulk

* Puranjay Mohan <p-mohan@ti.com> [220418 12:35]:
> From: Suman Anna <s-anna@ti.com>
> +/**
> + * pruss_cfg_ocp_master_ports() - configure PRUSS OCP master ports
> + * @pruss: the pruss instance handle
> + * @enable: set to true for enabling or false for disabling the OCP master ports
> + *
> + * This function programs the PRUSS_SYSCFG.STANDBY_INIT bit either to enable or
> + * disable the OCP master ports (applicable only on SoCs using OCP interconnect
> + * like the OMAP family). Clearing the bit achieves dual functionalities - one
> + * is to deassert the MStandby signal to the device PRCM, and the other is to
> + * enable OCP master ports to allow accesses outside of the PRU-ICSS. The
> + * function has to wait for the PRCM to acknowledge through the monitoring of
> + * the PRUSS_SYSCFG.SUB_MWAIT bit when enabling master ports. Setting the bit
> + * disables the master access, and also signals the PRCM that the PRUSS is ready
> + * for Standby.

Looks OK to me, some comments regarding runtime PM though for future patching
though.

Eventually we may want to handle this in drivers/bus/ti-sysc.c so it gets toggled
based on runtime PM. The PRUSS sysc register seems to be just a new variant of
sysc_regbits_omap4_simple with the standby and status bits added.

If using runtime PM for the PRUSS instance is not suitable for managing the
standby and status bits, then some comments should be added describing why
finer grained control is needed for these bits beyond runtime PM.

As far as I'm concerned, these can be done in separate changes, no need to update
this patch.

Regards,

Tony