lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZebFKlae0a-deBKl@hovoldconsulting.com>
Date: Tue, 5 Mar 2024 08:09:30 +0100
From: Johan Hovold <johan@...nel.org>
To: Krishna Kurapati <quic_kriskura@...cinc.com>
Cc: Bjorn Andersson <andersson@...nel.org>,
	Wesley Cheng <quic_wcheng@...cinc.com>,
	Konrad Dybcio <konrad.dybcio@...aro.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
	linux-arm-msm@...r.kernel.org, linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org, quic_ppratap@...cinc.com,
	quic_jackp@...cinc.com
Subject: Re: [PATCH] usb: dwc3: qcom: Remove ACPI support from glue driver

On Tue, Mar 05, 2024 at 09:51:43AM +0530, Krishna Kurapati wrote:
> Minimal ACPI support was added to the Qualcomm DWC3 glue driver in order to
> enable USB on SDM850 and SC8180X compute platforms. The support is still
> functional, but unnoticed regressions in other drivers indicates that no
> one actually booting any of platforms dependent on this implementation.
> 
> The functionality provides is the bare minimum and is not expected to aid
> in the effort of bringing full ACPI support to the driver in the future.
> 
> Remove the ACPI code from the Qualcomm DWC3 glue driver to aid in the
> implementation of improvements that are actually used like multiport and
> flattening device tree.

With a simple lookup function that returns the ACPI index based on name
this shouldn't be required to add multiport support even if it may
simplify it slightly. But IIRC it would help more with the devicetree
binding rework.
 
> Commit message by Bjorn Andersson.
> 
> Signed-off-by: Krishna Kurapati <quic_kriskura@...cinc.com>
> ---
>  drivers/usb/dwc3/dwc3-qcom.c | 273 ++---------------------------------
>  1 file changed, 11 insertions(+), 262 deletions(-)

You should update the Kconfig entry for USB_DWC3_QCOM as well and drop
the ACPI dependency.

>  static int dwc3_qcom_probe(struct platform_device *pdev)
>  {
>  	struct device_node	*np = pdev->dev.of_node;
>  	struct device		*dev = &pdev->dev;
>  	struct dwc3_qcom	*qcom;
>  	struct resource		*res, *parent_res = NULL;

You should drop parent_res as well.

> -	struct resource		local_res;
>  	int			ret, i;
>  	bool			ignore_pipe_clk;
>  	bool			wakeup_source;
> @@ -825,14 +659,6 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, qcom);
>  	qcom->dev = &pdev->dev;
>  
> -	if (has_acpi_companion(dev)) {
> -		qcom->acpi_pdata = acpi_device_get_match_data(dev);
> -		if (!qcom->acpi_pdata) {
> -			dev_err(&pdev->dev, "no supporting ACPI device data\n");
> -			return -EINVAL;
> -		}
> -	}
> -
>  	qcom->resets = devm_reset_control_array_get_optional_exclusive(dev);
>  	if (IS_ERR(qcom->resets)) {
>  		return dev_err_probe(&pdev->dev, PTR_ERR(qcom->resets),
> @@ -860,41 +686,18 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>  	}
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -
> -	if (np) {
> -		parent_res = res;
> -	} else {
> -		memcpy(&local_res, res, sizeof(struct resource));
> -		parent_res = &local_res;
> -
> -		parent_res->start = res->start +
> -			qcom->acpi_pdata->qscratch_base_offset;
> -		parent_res->end = parent_res->start +
> -			qcom->acpi_pdata->qscratch_base_size;
> -
> -		if (qcom->acpi_pdata->is_urs) {
> -			qcom->urs_usb = dwc3_qcom_create_urs_usb_platdev(dev);
> -			if (IS_ERR_OR_NULL(qcom->urs_usb)) {
> -				dev_err(dev, "failed to create URS USB platdev\n");
> -				if (!qcom->urs_usb)
> -					ret = -ENODEV;
> -				else
> -					ret = PTR_ERR(qcom->urs_usb);
> -				goto clk_disable;
> -			}
> -		}
> -	}
> +	parent_res = res;
>  
>  	qcom->qscratch_base = devm_ioremap_resource(dev, parent_res);

And just use res here.

>  	if (IS_ERR(qcom->qscratch_base)) {
>  		ret = PTR_ERR(qcom->qscratch_base);
> -		goto free_urs;
> +		goto clk_disable;
> }

Looks good to me otherwise.

Johan

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ