mbox series

[0/7] ti-sysc driver fix for hdq1w and few improvments

Message ID 20200221195256.54016-1-tony@atomide.com
Headers show
Series ti-sysc driver fix for hdq1w and few improvments | expand

Message

Tony Lindgren Feb. 21, 2020, 7:52 p.m. UTC
Hi all,

Here are some ti-sysc interconnect target module driver fixes and
improvments.

There's a fix for 1-wire reset, the rest can wait for v5.7 merge
window.

Regards,

Tony



Tony Lindgren (7):
  bus: ti-sysc: Fix 1-wire reset quirk
  bus: ti-sysc: Rename clk related quirks to pre_reset and post_reset
    quirks
  ti-sysc: Improve reset to work with modules with no sysconfig
  bus: ti-sysc: Consider non-existing registers too when matching quirks
  bus: ti-sysc: Don't warn about legacy property for nested ti-sysc
    devices
  bus: ti-sysc: Implement SoC revision handling
  bus: ti-sysc: Handle module unlock quirk needed for some RTC

 arch/arm/mach-omap2/pdata-quirks.c    |   6 +
 drivers/bus/ti-sysc.c                 | 430 ++++++++++++++++++++------
 include/linux/platform_data/ti-sysc.h |   2 +
 3 files changed, 348 insertions(+), 90 deletions(-)

Comments

Adam Ford March 2, 2022, 2:38 p.m. UTC | #1
On Fri, Feb 21, 2020 at 1:53 PM Tony Lindgren <tony@atomide.com> wrote:
>
> We need to know SoC type and features for cases where the same SoC
> may be installed in various versions on the same board and would need
> a separate dts file otherwise for the different variants.
>
> For example, am3703 is pin compatible with omap3630, but has sgx and
> iva accelerators disabled. We must not try to access the sgx or iva
> module registers on am3703, and need to set the unavailable devices
> disabled early.
>
> Let's also detect omap3430 as that is needed for display subsystem
> (DSS) reset later on, and GP vs EMU or HS devices. Further SoC
> specific disabled device detection can be added as needed, such as
> dra71x vs dra76x rtc and usb4.
>
> Cc: Adam Ford <aford173@gmail.com>
> Cc: André Hentschel <nerv@dawncrow.de>
> Cc: H. Nikolaus Schaller <hns@goldelico.com>
> Cc: Keerthy <j-keerthy@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>

Tony,

I apologize for digging up an old thread, but I finally managed to get
my hands on an OMAP3503.  It seems like older kernels do not boot at
all or hang somewhere in the boot process.  I am still seeing a
difference in behavior between OMAP3503 and OMAP3530, where 3505
throws a bunch of splat and a kernel panic, while the 3530 appears to
boot happily.

Using the latest 5.17-rc6, I had to remove some IVA and SGX references
from omap_l3_smx.h to get the 3503 to stop crashing on boot.

Do you have any ideas how we can make the omap3_l3_app_bases and
omap3_l3_debug_bases more cleanly remove the IVA and SGX references
if/when OMAP3503 is detected?  I assume the same algorithm would have
to detect a AM3703 as well.  I'm trying to get my hands on an AM3703
to see if there is similar behavior as what I'm seeing on the
OMAP3503.

Thanks,

adam

> ---
>  arch/arm/mach-omap2/pdata-quirks.c    |   6 +
>  drivers/bus/ti-sysc.c                 | 194 +++++++++++++++++++++++++-
>  include/linux/platform_data/ti-sysc.h |   1 +
>  3 files changed, 200 insertions(+), 1 deletion(-)
>
> 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
> @@ -397,10 +397,16 @@ static int ti_sysc_shutdown_module(struct device *dev,
>         return omap_hwmod_shutdown(cookie->data);
>  }
>
> +static bool ti_sysc_soc_type_gp(void)
> +{
> +       return omap_type() == OMAP2_DEVICE_TYPE_GP;
> +}
> +
>  static struct of_dev_auxdata omap_auxdata_lookup[];
>
>  static struct ti_sysc_platform_data ti_sysc_pdata = {
>         .auxdata = omap_auxdata_lookup,
> +       .soc_type_gp = ti_sysc_soc_type_gp,
>         .init_clockdomain = ti_sysc_clkdm_init,
>         .clkdm_deny_idle = ti_sysc_clkdm_deny_idle,
>         .clkdm_allow_idle = ti_sysc_clkdm_allow_idle,
> diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
> --- a/drivers/bus/ti-sysc.c
> +++ b/drivers/bus/ti-sysc.c
> @@ -7,6 +7,7 @@
>  #include <linux/clk.h>
>  #include <linux/clkdev.h>
>  #include <linux/delay.h>
> +#include <linux/list.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_domain.h>
> @@ -15,15 +16,47 @@
>  #include <linux/of_address.h>
>  #include <linux/of_platform.h>
>  #include <linux/slab.h>
> +#include <linux/sys_soc.h>
>  #include <linux/iopoll.h>
>
>  #include <linux/platform_data/ti-sysc.h>
>
>  #include <dt-bindings/bus/ti-sysc.h>
>
> +#define DIS_ISP                BIT(2)
> +#define DIS_IVA                BIT(1)
> +#define DIS_SGX                BIT(0)
> +
> +#define SOC_FLAG(match, flag)  { .machine = match, .data = (void *)(flag), }
> +
>  #define MAX_MODULE_SOFTRESET_WAIT              10000
>
> -static const char * const reg_names[] = { "rev", "sysc", "syss", };
> +enum sysc_soc {
> +       SOC_UNKNOWN,
> +       SOC_2420,
> +       SOC_2430,
> +       SOC_3430,
> +       SOC_3630,
> +       SOC_4430,
> +       SOC_4460,
> +       SOC_4470,
> +       SOC_5430,
> +       SOC_AM3,
> +       SOC_AM4,
> +       SOC_DRA7,
> +};
> +
> +struct sysc_address {
> +       unsigned long base;
> +       struct list_head node;
> +};
> +
> +struct sysc_soc_info {
> +       unsigned long general_purpose:1;
> +       enum sysc_soc soc;
> +       struct mutex list_lock;                 /* disabled modules list lock */
> +       struct list_head disabled_modules;
> +};
>
>  enum sysc_clocks {
>         SYSC_FCK,
> @@ -39,6 +72,8 @@ enum sysc_clocks {
>         SYSC_MAX_CLOCKS,
>  };
>
> +static struct sysc_soc_info *sysc_soc;
> +static const char * const reg_names[] = { "rev", "sysc", "syss", };
>  static const char * const clock_names[SYSC_MAX_CLOCKS] = {
>         "fck", "ick", "opt0", "opt1", "opt2", "opt3", "opt4",
>         "opt5", "opt6", "opt7",
> @@ -2382,6 +2417,154 @@ static void ti_sysc_idle(struct work_struct *work)
>                 pm_runtime_put_sync(ddata->dev);
>  }
>
> +/*
> + * SoC model and features detection. Only needed for SoCs that need
> + * special handling for quirks, no need to list others.
> + */
> +static const struct soc_device_attribute sysc_soc_match[] = {
> +       SOC_FLAG("OMAP242*", SOC_2420),
> +       SOC_FLAG("OMAP243*", SOC_2430),
> +       SOC_FLAG("OMAP3[45]*", SOC_3430),
> +       SOC_FLAG("OMAP3[67]*", SOC_3630),
> +       SOC_FLAG("OMAP443*", SOC_4430),
> +       SOC_FLAG("OMAP446*", SOC_4460),
> +       SOC_FLAG("OMAP447*", SOC_4470),
> +       SOC_FLAG("OMAP54*", SOC_5430),
> +       SOC_FLAG("AM433", SOC_AM3),
> +       SOC_FLAG("AM43*", SOC_AM4),
> +       SOC_FLAG("DRA7*", SOC_DRA7),
> +
> +       { /* sentinel */ },
> +};
> +
> +/*
> + * List of SoCs variants with disabled features. By default we assume all
> + * devices in the device tree are available so no need to list those SoCs.
> + */
> +static const struct soc_device_attribute sysc_soc_feat_match[] = {
> +       /* OMAP3430/3530 and AM3517 variants with some accelerators disabled */
> +       SOC_FLAG("AM3505", DIS_SGX),
> +       SOC_FLAG("OMAP3525", DIS_SGX),
> +       SOC_FLAG("OMAP3515", DIS_IVA | DIS_SGX),
> +       SOC_FLAG("OMAP3503", DIS_ISP | DIS_IVA | DIS_SGX),
> +
> +       /* OMAP3630/DM3730 variants with some accelerators disabled */
> +       SOC_FLAG("AM3703", DIS_IVA | DIS_SGX),
> +       SOC_FLAG("DM3725", DIS_SGX),
> +       SOC_FLAG("OMAP3611", DIS_ISP | DIS_IVA | DIS_SGX),
> +       SOC_FLAG("OMAP3615/AM3715", DIS_IVA),
> +       SOC_FLAG("OMAP3621", DIS_ISP),
> +
> +       { /* sentinel */ },
> +};
> +
> +static int sysc_add_disabled(unsigned long base)
> +{
> +       struct sysc_address *disabled_module;
> +
> +       disabled_module = kzalloc(sizeof(*disabled_module), GFP_KERNEL);
> +       if (!disabled_module)
> +               return -ENOMEM;
> +
> +       disabled_module->base = base;
> +
> +       mutex_lock(&sysc_soc->list_lock);
> +       list_add(&disabled_module->node, &sysc_soc->disabled_modules);
> +       mutex_unlock(&sysc_soc->list_lock);
> +
> +       return 0;
> +}
> +
> +/*
> + * One time init to detect the booted SoC and disable unavailable features.
> + * Note that we initialize static data shared across all ti-sysc instances
> + * so ddata is only used for SoC type. This can be called from module_init
> + * once we no longer need to rely on platform data.
> + */
> +static int sysc_init_soc(struct sysc *ddata)
> +{
> +       const struct soc_device_attribute *match;
> +       struct ti_sysc_platform_data *pdata;
> +       unsigned long features = 0;
> +
> +       if (sysc_soc)
> +               return 0;
> +
> +       sysc_soc = kzalloc(sizeof(*sysc_soc), GFP_KERNEL);
> +       if (!sysc_soc)
> +               return -ENOMEM;
> +
> +       mutex_init(&sysc_soc->list_lock);
> +       INIT_LIST_HEAD(&sysc_soc->disabled_modules);
> +       sysc_soc->general_purpose = true;
> +
> +       pdata = dev_get_platdata(ddata->dev);
> +       if (pdata && pdata->soc_type_gp)
> +               sysc_soc->general_purpose = pdata->soc_type_gp();
> +
> +       match = soc_device_match(sysc_soc_match);
> +       if (match && match->data)
> +               sysc_soc->soc = (int)match->data;
> +
> +       match = soc_device_match(sysc_soc_feat_match);
> +       if (!match)
> +               return 0;
> +
> +       if (match->data)
> +               features = (unsigned long)match->data;
> +
> +       /*
> +        * Add disabled devices to the list based on the module base.
> +        * Note that this must be done before we attempt to access the
> +        * device and have module revision checks working.
> +        */
> +       if (features & DIS_ISP)
> +               sysc_add_disabled(0x480bd400);
> +       if (features & DIS_IVA)
> +               sysc_add_disabled(0x5d000000);
> +       if (features & DIS_SGX)
> +               sysc_add_disabled(0x50000000);
> +
> +       return 0;
> +}
> +
> +static void sysc_cleanup_soc(void)
> +{
> +       struct sysc_address *disabled_module;
> +       struct list_head *pos, *tmp;
> +
> +       if (!sysc_soc)
> +               return;
> +
> +       mutex_lock(&sysc_soc->list_lock);
> +       list_for_each_safe(pos, tmp, &sysc_soc->disabled_modules) {
> +               disabled_module = list_entry(pos, struct sysc_address, node);
> +               list_del(pos);
> +               kfree(disabled_module);
> +       }
> +       mutex_unlock(&sysc_soc->list_lock);
> +}
> +
> +static int sysc_check_disabled_devices(struct sysc *ddata)
> +{
> +       struct sysc_address *disabled_module;
> +       struct list_head *pos;
> +       int error = 0;
> +
> +       mutex_lock(&sysc_soc->list_lock);
> +       list_for_each(pos, &sysc_soc->disabled_modules) {
> +               disabled_module = list_entry(pos, struct sysc_address, node);
> +               if (ddata->module_pa == disabled_module->base) {
> +                       dev_dbg(ddata->dev, "module disabled for this SoC\n");
> +                       error = -ENODEV;
> +                       break;
> +               }
> +       }
> +       mutex_unlock(&sysc_soc->list_lock);
> +
> +       return error;
> +}
> +
>  static const struct of_device_id sysc_match_table[] = {
>         { .compatible = "simple-bus", },
>         { /* sentinel */ },
> @@ -2400,6 +2583,10 @@ static int sysc_probe(struct platform_device *pdev)
>         ddata->dev = &pdev->dev;
>         platform_set_drvdata(pdev, ddata);
>
> +       error = sysc_init_soc(ddata);
> +       if (error)
> +               return error;
> +
>         error = sysc_init_match(ddata);
>         if (error)
>                 return error;
> @@ -2430,6 +2617,10 @@ static int sysc_probe(struct platform_device *pdev)
>
>         sysc_init_early_quirks(ddata);
>
> +       error = sysc_check_disabled_devices(ddata);
> +       if (error)
> +               return error;
> +
>         error = sysc_get_clocks(ddata);
>         if (error)
>                 return error;
> @@ -2560,6 +2751,7 @@ static void __exit sysc_exit(void)
>  {
>         bus_unregister_notifier(&platform_bus_type, &sysc_nb);
>         platform_driver_unregister(&sysc_driver);
> +       sysc_cleanup_soc();
>  }
>  module_exit(sysc_exit);
>
> diff --git a/include/linux/platform_data/ti-sysc.h b/include/linux/platform_data/ti-sysc.h
> --- a/include/linux/platform_data/ti-sysc.h
> +++ b/include/linux/platform_data/ti-sysc.h
> @@ -141,6 +141,7 @@ struct clk;
>
>  struct ti_sysc_platform_data {
>         struct of_dev_auxdata *auxdata;
> +       bool (*soc_type_gp)(void);
>         int (*init_clockdomain)(struct device *dev, struct clk *fck,
>                                 struct clk *ick, struct ti_sysc_cookie *cookie);
>         void (*clkdm_deny_idle)(struct device *dev,
> --
> 2.25.1
Tony Lindgren March 4, 2022, 6:56 a.m. UTC | #2
Hi,

* Adam Ford <aford173@gmail.com> [220302 14:37]:
> I apologize for digging up an old thread, but I finally managed to get
> my hands on an OMAP3503.  It seems like older kernels do not boot at
> all or hang somewhere in the boot process.  I am still seeing a
> difference in behavior between OMAP3503 and OMAP3530, where 3505
> throws a bunch of splat and a kernel panic, while the 3530 appears to
> boot happily.
> 
> Using the latest 5.17-rc6, I had to remove some IVA and SGX references
> from omap_l3_smx.h to get the 3503 to stop crashing on boot.

OK interesting, I did not know those registers are not accessible
on 3503.

> Do you have any ideas how we can make the omap3_l3_app_bases and
> omap3_l3_debug_bases more cleanly remove the IVA and SGX references
> if/when OMAP3503 is detected?  I assume the same algorithm would have
> to detect a AM3703 as well.  I'm trying to get my hands on an AM3703
> to see if there is similar behavior as what I'm seeing on the
> OMAP3503.

As there are possibly multiple omap3 variants used on the same
boards, we need to rely on the runtime detection of the SoC.
So yeah soc_device_attribute is the way to go here.

I don't recall any similar issues booting 3703 but it's been a while
so worth testing for sure.

Regards,

Tony
Adam Ford March 4, 2022, 1:57 p.m. UTC | #3
On Fri, Mar 4, 2022 at 12:57 AM Tony Lindgren <tony@atomide.com> wrote:
>
> Hi,
>
> * Adam Ford <aford173@gmail.com> [220302 14:37]:
> > I apologize for digging up an old thread, but I finally managed to get
> > my hands on an OMAP3503.  It seems like older kernels do not boot at
> > all or hang somewhere in the boot process.  I am still seeing a
> > difference in behavior between OMAP3503 and OMAP3530, where 3505
> > throws a bunch of splat and a kernel panic, while the 3530 appears to
> > boot happily.
> >
> > Using the latest 5.17-rc6, I had to remove some IVA and SGX references
> > from omap_l3_smx.h to get the 3503 to stop crashing on boot.
>
> OK interesting, I did not know those registers are not accessible
> on 3503.
>
> > Do you have any ideas how we can make the omap3_l3_app_bases and
> > omap3_l3_debug_bases more cleanly remove the IVA and SGX references
> > if/when OMAP3503 is detected?  I assume the same algorithm would have
> > to detect a AM3703 as well.  I'm trying to get my hands on an AM3703
> > to see if there is similar behavior as what I'm seeing on the
> > OMAP3503.
>
> As there are possibly multiple omap3 variants used on the same
> boards, we need to rely on the runtime detection of the SoC.
> So yeah soc_device_attribute is the way to go here.
>
> I don't recall any similar issues booting 3703 but it's been a while
> so worth testing for sure.

In addition to the OMAP3503, I managed to get an AM3703.  From what I
can tell, going back as far as kernel 4.9, the OMAP3503 does not boot
at all. I haven't tried the 4.4 since it's marked EOL at this point.

I have not started testing the AM3703 yet, but I think it would be a
good idea to backport this to stable at some point, since it appears
to fix a serious regression,  not booting.  I'm going to work on some
experiments with both the AM3703 and OMAP3503 to see what works, what
doesn't and I'm going try to come up with some ideas on how to address
the omap3_l3_app changes, but if you have any ideas on how to do it
cleanly, I'm open for suggestions.

adam
>
> Regards,
>
> Tony