[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-omap
Subject:    Re: [PATCH] remoteproc/wkup_m3: Drop legacy platform data no longer needed
From:       Suman Anna <s-anna () ti ! com>
Date:       2021-01-28 23:14:38
Message-ID: ec2b5581-b207-f7ed-2f0d-09e5b5d9dfaa () ti ! com
[Download RAW message or body]

Hi Tony,

On 1/28/21 2:44 AM, Tony Lindgren wrote:
> We have v5.11 booting am3 and 4 with ti-sysc interconnect target module
> driver and genpd. As part of that conversion, wkup_m3 driver got converted
> to optionally use reset driver instead of legacy platform data with
> commit 57df7e370d2a ("remoteproc/wkup_m3: Use reset control driver if
> available").
> 
> The related SoC calls already got removed with commit b62168e516da ("ARM:
> OMAP2+: Fix am4 only build after genpd changes").
> 
> We can now just drop the legacy platform data for v5.12 or later, there's
> no rush to do this for v5.11.
> 
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Dave Gerlach <d-gerlach@ti.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Suman Anna <s-anna@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>  arch/arm/mach-omap2/pdata-quirks.c    |  1 -
>  drivers/remoteproc/wkup_m3_rproc.c    | 37 ++-------------------------
>  include/linux/platform_data/wkup_m3.h | 22 ----------------
>  3 files changed, 2 insertions(+), 58 deletions(-)
>  delete mode 100644 include/linux/platform_data/wkup_m3.h
> 
> diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c
> --- a/arch/arm/mach-omap2/pdata-quirks.c
> +++ b/arch/arm/mach-omap2/pdata-quirks.c
> @@ -21,7 +21,6 @@
>  #include <linux/platform_data/hsmmc-omap.h>
>  #include <linux/platform_data/iommu-omap.h>
>  #include <linux/platform_data/ti-sysc.h>
> -#include <linux/platform_data/wkup_m3.h>
>  #include <linux/platform_data/asoc-ti-mcbsp.h>
>  #include <linux/platform_data/ti-prm.h>
>  
> diff --git a/drivers/remoteproc/wkup_m3_rproc.c b/drivers/remoteproc/wkup_m3_rproc.c
> --- a/drivers/remoteproc/wkup_m3_rproc.c
> +++ b/drivers/remoteproc/wkup_m3_rproc.c
> @@ -19,8 +19,6 @@
>  #include <linux/remoteproc.h>
>  #include <linux/reset.h>
>  
> -#include <linux/platform_data/wkup_m3.h>
> -
>  #include "remoteproc_internal.h"
>  
>  #define WKUPM3_MEM_MAX	2
> @@ -56,37 +54,15 @@ struct wkup_m3_rproc {
>  static int wkup_m3_rproc_start(struct rproc *rproc)
>  {
>  	struct wkup_m3_rproc *wkupm3 = rproc->priv;
> -	struct platform_device *pdev = wkupm3->pdev;
> -	struct device *dev = &pdev->dev;
> -	struct wkup_m3_platform_data *pdata = dev_get_platdata(dev);
> -	int error = 0;
> -
> -	error = reset_control_deassert(wkupm3->rsts);
>  
> -	if (!wkupm3->rsts && pdata->deassert_reset(pdev, pdata->reset_name)) {
> -		dev_err(dev, "Unable to reset wkup_m3!\n");
> -		error = -ENODEV;
> -	}
> -
> -	return error;
> +	return reset_control_deassert(wkupm3->rsts);
>  }
>  
>  static int wkup_m3_rproc_stop(struct rproc *rproc)
>  {
>  	struct wkup_m3_rproc *wkupm3 = rproc->priv;
> -	struct platform_device *pdev = wkupm3->pdev;
> -	struct device *dev = &pdev->dev;
> -	struct wkup_m3_platform_data *pdata = dev_get_platdata(dev);
> -	int error = 0;
>  
> -	error = reset_control_assert(wkupm3->rsts);
> -
> -	if (!wkupm3->rsts && pdata->assert_reset(pdev, pdata->reset_name)) {
> -		dev_err(dev, "Unable to assert reset of wkup_m3!\n");
> -		error = -ENODEV;
> -	}
> -
> -	return error;
> +	return reset_control_assert(wkupm3->rsts);
>  }
>  
>  static void *wkup_m3_rproc_da_to_va(struct rproc *rproc, u64 da, size_t len)
> @@ -128,7 +104,6 @@ MODULE_DEVICE_TABLE(of, wkup_m3_rproc_of_match);
>  static int wkup_m3_rproc_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> -	struct wkup_m3_platform_data *pdata = dev->platform_data;
>  	/* umem always needs to be processed first */
>  	const char *mem_names[WKUPM3_MEM_MAX] = { "umem", "dmem" };
>  	struct wkup_m3_rproc *wkupm3;
> @@ -171,14 +146,6 @@ static int wkup_m3_rproc_probe(struct platform_device *pdev)
>  	wkupm3->rsts = devm_reset_control_get_optional_shared(dev, "rstctrl");

We should no longer be using the optional variant now that this is what is
expected. Otherwise, rest of the changes look good. Any reason to use the shared
variant, I need to take a closer look at how the resets are represented.

Do I need anything else or I should be able to test this patch on latest 5.11-rc
or master?

regards
Suman

>  	if (IS_ERR(wkupm3->rsts))
>  		return PTR_ERR(wkupm3->rsts);
> -	if (!wkupm3->rsts) {
> -		if (!(pdata && pdata->deassert_reset && pdata->assert_reset &&
> -		      pdata->reset_name)) {
> -			dev_err(dev, "Platform data missing!\n");
> -			ret = -ENODEV;
> -			goto err_put_rproc;
> -		}
> -	}
>  
>  	for (i = 0; i < ARRAY_SIZE(mem_names); i++) {
>  		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> diff --git a/include/linux/platform_data/wkup_m3.h b/include/linux/platform_data/wkup_m3.h
> deleted file mode 100644
> --- a/include/linux/platform_data/wkup_m3.h
> +++ /dev/null
> @@ -1,22 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -/*
> - * TI Wakeup M3 remote processor platform data
> - *
> - * Copyright (C) 2014-2015 Texas Instruments, Inc.
> - *
> - * Dave Gerlach <d-gerlach@ti.com>
> - */
> -
> -#ifndef _LINUX_PLATFORM_DATA_WKUP_M3_H
> -#define _LINUX_PLATFORM_DATA_WKUP_M3_H
> -
> -struct platform_device;
> -
> -struct wkup_m3_platform_data {
> -	const char *reset_name;
> -
> -	int (*assert_reset)(struct platform_device *pdev, const char *name);
> -	int (*deassert_reset)(struct platform_device *pdev, const char *name);
> -};
> -
> -#endif /* _LINUX_PLATFORM_DATA_WKUP_M3_H */
> 

[prev in list] [next in list] [prev in thread] [next in thread]