Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
Date:   Tue, 24 Oct 2023 09:03:08 +0200
From:   Johan Hovold <johan@kernel.org>
To:     Krishna Kurapati PSSNV <quic_kriskura@quicinc.com>
Cc:     Thinh Nguyen <Thinh.Nguyen@synopsys.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Philipp Zabel <p.zabel@pengutronix.de>,
        Andy Gross <agross@kernel.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Felipe Balbi <balbi@kernel.org>,
        Wesley Cheng <quic_wcheng@quicinc.com>,
        linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
        quic_pkondeti@quicinc.com, quic_ppratap@quicinc.com,
        quic_jackp@quicinc.com, ahalaney@redhat.com,
        quic_shazhuss@quicinc.com
Subject: Re: [PATCH v13 07/10] usb: dwc3: qcom: Add multiport suspend/resume
 support for wrapper
Message-ID: <ZTdsLMT9PNcnroHC@hovoldconsulting.com>
References: <20231007154806.605-1-quic_kriskura@quicinc.com>
 <20231007154806.605-8-quic_kriskura@quicinc.com>
 <ZTaYNjRyT1Fn4QWX@hovoldconsulting.com>
 <73168f4b-0dc2-4060-99f2-c5e9973dbf52@quicinc.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <73168f4b-0dc2-4060-99f2-c5e9973dbf52@quicinc.com>
Precedence: bulk

On Mon, Oct 23, 2023 at 10:52:38PM +0530, Krishna Kurapati PSSNV wrote:
> On 10/23/2023 9:28 PM, Johan Hovold wrote:
> > On Sat, Oct 07, 2023 at 09:18:03PM +0530, Krishna Kurapati wrote:
  
> >> -#define PWR_EVNT_IRQ_STAT_REG			0x58
> >> +#define PWR_EVNT_IRQ1_STAT_REG			0x58
> >> +#define PWR_EVNT_IRQ2_STAT_REG			0x1dc
> >> +#define PWR_EVNT_IRQ3_STAT_REG			0x228
> >> +#define PWR_EVNT_IRQ4_STAT_REG			0x238
> > 
> > Not sure these defines makes sense on their own. You now only use them
> > via the array below.
> > 
> > I think I already asked you whether these offsets depend on SoC and you
> > said no, right?
> > 
> There are only 3 QC SoC's today that support multiport.
> The offsets mentioned here are for SC8280 based platforms.
> 
> For Sc8180 based platforms, these are the offsets:
> USB3_MP_PWR_EVNT_IRQ_STAT	0xA4F8858
> USB3_MP_PWR_EVNT_IRQ_1_STAT	0xA4F89DC
> 
> These would translate to 0x58 and 0x1DC
> 
> And for SX8380 the values are as follows:
> 
> USB3_MP_PWR_EVNT_IRQ_STAT	0xA4F8858
> USB3_MP_PWR_EVNT_IRQ_1_STAT	0xA4F89DC
> 
> So here also, the offsets are same. 0x58 and 0x1DC.
> So these are not SoC specific (atleast looking at the controllers 
> present). But there is no mathematical pattern to denote this as in the 
> following form (x + (port_num) * y). So made an array like this.

Sounds good. Thanks for confirming.

> >> +#define NUM_PWR_EVENT_STAT_REGS	4
> > 
> > You already have MAX_PORTS, why are you defining a new define that will
> > always have to be equal to MAX_PORTS?
> > 
> Do you recommend using the same max_ports ? If so, I can remove this 
> macro altogether.

Indeed, and perhaps also some (compile-time) assert as the driver breaks
if they ever get out of sync.

Johan