All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tero Kristo <t-kristo@ti.com>
To: Adam Ford <aford173@gmail.com>, Tony Lindgren <tony@atomide.com>
Cc: <linux-clk@vger.kernel.org>,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@codeaurora.org>, <linux-omap@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCHv2 15/15] clk: ti: convert to use proper register definition for all accesses
Date: Thu, 18 Jan 2018 09:34:37 +0200	[thread overview]
Message-ID: <4f0f185a-2890-3747-7060-1f5b5a77022a@ti.com> (raw)
In-Reply-To: <CAHCN7xJ-NaKoeZOqrdc48fFCLNQvm_uRXC0ErtiG+Ac86a14+w@mail.gmail.com>

On 17/01/18 23:44, Adam Ford wrote:
> On Wed, Jan 17, 2018 at 3:19 PM, Tony Lindgren <tony@atomide.com> wrote:
>> * Adam Ford <aford173@gmail.com> [180117 15:15]:
>>> On Wed, Jan 17, 2018 at 8:02 AM, Tero Kristo <t-kristo@ti.com> wrote:
>>>> On 17/01/18 15:27, Adam Ford wrote:
>>>>>
>>>>> On Sat, Mar 11, 2017 at 6:50 AM, Tero Kristo <t-kristo@ti.com> wrote:
>>>>>>
>>>>>> Currently, TI clock driver uses an encapsulated struct that is cast into
>>>>>> a void pointer to store all register addresses. This can be considered
>>>>>> as rather nasty hackery, and prevents from expanding the register
>>>>>> address field also. Instead, replace all the code to use proper struct
>>>>>> in place for this, which contains all the previously used data.
>>>>>>
>>>>>> This patch is rather large as it is touching multiple files, but this
>>>>>> can't be split up as we need to avoid any boot breakage.
>>>>>>
>>>>>
>>>>> I know it's late coming, but according to git bisect, this patch is
>>>>> causing some problems with Logic PD Torpedo 37xx Dev kit.
>>>>
>>>>
>>>> Oh reporting bugs is never too late, thanks for posting this out.
>>>>
>>>>>
>>>>> It it is a DM3730 that has a WL1283 chipset attached to the SDIO
>>>>> interface on MMC3.  The driver seems to load properly, but when
>>>>> loading wpa_supplicant to activate the WL1283, we get a giant crash.
>>>>> I checked kernel revisions starting at 4.14 and working back to when
>>>>> it worked, then used git bisect from there.
>>>>>
>>>>> I am hoping it might be a simple fix for something that just needs to
>>>>> get added or tweaked in the device tree.
>>>>
>>>>
>>>> I don't have access to the specific hw, but can you try to dig out which
>>>> hwmod is causing the crash? Just print out the oh->name from the
>>>> _wait_softreset_complete. That would help root causing the issue.
>>>>
>>>
>>> With one small patch, I was able to make it work again.
>>>
>>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
>>> index 2dbd632..ed1f625 100644
>>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>>> @@ -477,7 +477,7 @@ static int _wait_softreset_complete(struct omap_hwmod *oh)
>>>          int c = 0;
>>>
>>>          sysc = oh->class->sysc;
>>> -
>>> +pr_warn("_wait_softreset_complete: %s\n", oh->name);
>>>          if (sysc->sysc_flags & SYSS_HAS_RESET_STATUS)
>>>                  omap_test_timeout((omap_hwmod_read(oh, sysc->syss_offs)
>>>                                     & SYSS_RESETDONE_MASK),
>>>
>>>
>>> This leads me to believe that the omap_test_timeout functions might
>>> not be working quite right.
>>
>> There may be a srst_udelay needed for some module, see commit
>> ebf244148092 ("ARM: OMAP2+: Use srst_udelay for USB on dm814x")
>> for example.
>>
>> You might be able to find which module it is by commenting out
>> postcore_initcall_sync(omap3_l3_init) in drivers/bus/omap_l3_smx.c
>> temporarily as the system will most likely hang right there.
> 
> I commented out that line as you suggested, but the system boots as
> normal and I get the crash (as normal)
> 
> I am looking through the DM3730 and OMAP3630 TRM now.  Any thought on
> a keyword search I should use to see which hwmods might require
> srst_udelay?

Looking at the log you provided, it looks like only mmc1, mmc3 and i2c1 
are reset during the wlan probe. Based on the prints coming out in the 
failing case, it looks like the culprit might be mmc3 for some reason.

[   18.239746] _wait_softreset_complete: mmc3
[   18.638580] _wait_softreset_complete: mmc3
[   18.657562] _wait_softreset_complete: mmc1
[   18.833374] wlcore: firmware booted (Rev 7.3.10.0.141)

^ the firmware notification above does not come out in the crash.

-Tero

> 
> adam
> 
>>
>> Regards,
>>
>> Tony
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

WARNING: multiple messages have this Message-ID (diff)
From: Tero Kristo <t-kristo@ti.com>
To: Adam Ford <aford173@gmail.com>, Tony Lindgren <tony@atomide.com>
Cc: linux-clk@vger.kernel.org,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@codeaurora.org>,
	linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCHv2 15/15] clk: ti: convert to use proper register definition for all accesses
Date: Thu, 18 Jan 2018 09:34:37 +0200	[thread overview]
Message-ID: <4f0f185a-2890-3747-7060-1f5b5a77022a@ti.com> (raw)
In-Reply-To: <CAHCN7xJ-NaKoeZOqrdc48fFCLNQvm_uRXC0ErtiG+Ac86a14+w@mail.gmail.com>

On 17/01/18 23:44, Adam Ford wrote:
> On Wed, Jan 17, 2018 at 3:19 PM, Tony Lindgren <tony@atomide.com> wrote:
>> * Adam Ford <aford173@gmail.com> [180117 15:15]:
>>> On Wed, Jan 17, 2018 at 8:02 AM, Tero Kristo <t-kristo@ti.com> wrote:
>>>> On 17/01/18 15:27, Adam Ford wrote:
>>>>>
>>>>> On Sat, Mar 11, 2017 at 6:50 AM, Tero Kristo <t-kristo@ti.com> wrote:
>>>>>>
>>>>>> Currently, TI clock driver uses an encapsulated struct that is cast into
>>>>>> a void pointer to store all register addresses. This can be considered
>>>>>> as rather nasty hackery, and prevents from expanding the register
>>>>>> address field also. Instead, replace all the code to use proper struct
>>>>>> in place for this, which contains all the previously used data.
>>>>>>
>>>>>> This patch is rather large as it is touching multiple files, but this
>>>>>> can't be split up as we need to avoid any boot breakage.
>>>>>>
>>>>>
>>>>> I know it's late coming, but according to git bisect, this patch is
>>>>> causing some problems with Logic PD Torpedo 37xx Dev kit.
>>>>
>>>>
>>>> Oh reporting bugs is never too late, thanks for posting this out.
>>>>
>>>>>
>>>>> It it is a DM3730 that has a WL1283 chipset attached to the SDIO
>>>>> interface on MMC3.  The driver seems to load properly, but when
>>>>> loading wpa_supplicant to activate the WL1283, we get a giant crash.
>>>>> I checked kernel revisions starting at 4.14 and working back to when
>>>>> it worked, then used git bisect from there.
>>>>>
>>>>> I am hoping it might be a simple fix for something that just needs to
>>>>> get added or tweaked in the device tree.
>>>>
>>>>
>>>> I don't have access to the specific hw, but can you try to dig out which
>>>> hwmod is causing the crash? Just print out the oh->name from the
>>>> _wait_softreset_complete. That would help root causing the issue.
>>>>
>>>
>>> With one small patch, I was able to make it work again.
>>>
>>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
>>> index 2dbd632..ed1f625 100644
>>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>>> @@ -477,7 +477,7 @@ static int _wait_softreset_complete(struct omap_hwmod *oh)
>>>          int c = 0;
>>>
>>>          sysc = oh->class->sysc;
>>> -
>>> +pr_warn("_wait_softreset_complete: %s\n", oh->name);
>>>          if (sysc->sysc_flags & SYSS_HAS_RESET_STATUS)
>>>                  omap_test_timeout((omap_hwmod_read(oh, sysc->syss_offs)
>>>                                     & SYSS_RESETDONE_MASK),
>>>
>>>
>>> This leads me to believe that the omap_test_timeout functions might
>>> not be working quite right.
>>
>> There may be a srst_udelay needed for some module, see commit
>> ebf244148092 ("ARM: OMAP2+: Use srst_udelay for USB on dm814x")
>> for example.
>>
>> You might be able to find which module it is by commenting out
>> postcore_initcall_sync(omap3_l3_init) in drivers/bus/omap_l3_smx.c
>> temporarily as the system will most likely hang right there.
> 
> I commented out that line as you suggested, but the system boots as
> normal and I get the crash (as normal)
> 
> I am looking through the DM3730 and OMAP3630 TRM now.  Any thought on
> a keyword search I should use to see which hwmods might require
> srst_udelay?

Looking at the log you provided, it looks like only mmc1, mmc3 and i2c1 
are reset during the wlan probe. Based on the prints coming out in the 
failing case, it looks like the culprit might be mmc3 for some reason.

[   18.239746] _wait_softreset_complete: mmc3
[   18.638580] _wait_softreset_complete: mmc3
[   18.657562] _wait_softreset_complete: mmc1
[   18.833374] wlcore: firmware booted (Rev 7.3.10.0.141)

^ the firmware notification above does not come out in the crash.

-Tero

> 
> adam
> 
>>
>> Regards,
>>
>> Tony
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

WARNING: multiple messages have this Message-ID (diff)
From: t-kristo@ti.com (Tero Kristo)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCHv2 15/15] clk: ti: convert to use proper register definition for all accesses
Date: Thu, 18 Jan 2018 09:34:37 +0200	[thread overview]
Message-ID: <4f0f185a-2890-3747-7060-1f5b5a77022a@ti.com> (raw)
In-Reply-To: <CAHCN7xJ-NaKoeZOqrdc48fFCLNQvm_uRXC0ErtiG+Ac86a14+w@mail.gmail.com>

On 17/01/18 23:44, Adam Ford wrote:
> On Wed, Jan 17, 2018 at 3:19 PM, Tony Lindgren <tony@atomide.com> wrote:
>> * Adam Ford <aford173@gmail.com> [180117 15:15]:
>>> On Wed, Jan 17, 2018 at 8:02 AM, Tero Kristo <t-kristo@ti.com> wrote:
>>>> On 17/01/18 15:27, Adam Ford wrote:
>>>>>
>>>>> On Sat, Mar 11, 2017 at 6:50 AM, Tero Kristo <t-kristo@ti.com> wrote:
>>>>>>
>>>>>> Currently, TI clock driver uses an encapsulated struct that is cast into
>>>>>> a void pointer to store all register addresses. This can be considered
>>>>>> as rather nasty hackery, and prevents from expanding the register
>>>>>> address field also. Instead, replace all the code to use proper struct
>>>>>> in place for this, which contains all the previously used data.
>>>>>>
>>>>>> This patch is rather large as it is touching multiple files, but this
>>>>>> can't be split up as we need to avoid any boot breakage.
>>>>>>
>>>>>
>>>>> I know it's late coming, but according to git bisect, this patch is
>>>>> causing some problems with Logic PD Torpedo 37xx Dev kit.
>>>>
>>>>
>>>> Oh reporting bugs is never too late, thanks for posting this out.
>>>>
>>>>>
>>>>> It it is a DM3730 that has a WL1283 chipset attached to the SDIO
>>>>> interface on MMC3.  The driver seems to load properly, but when
>>>>> loading wpa_supplicant to activate the WL1283, we get a giant crash.
>>>>> I checked kernel revisions starting at 4.14 and working back to when
>>>>> it worked, then used git bisect from there.
>>>>>
>>>>> I am hoping it might be a simple fix for something that just needs to
>>>>> get added or tweaked in the device tree.
>>>>
>>>>
>>>> I don't have access to the specific hw, but can you try to dig out which
>>>> hwmod is causing the crash? Just print out the oh->name from the
>>>> _wait_softreset_complete. That would help root causing the issue.
>>>>
>>>
>>> With one small patch, I was able to make it work again.
>>>
>>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
>>> index 2dbd632..ed1f625 100644
>>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>>> @@ -477,7 +477,7 @@ static int _wait_softreset_complete(struct omap_hwmod *oh)
>>>          int c = 0;
>>>
>>>          sysc = oh->class->sysc;
>>> -
>>> +pr_warn("_wait_softreset_complete: %s\n", oh->name);
>>>          if (sysc->sysc_flags & SYSS_HAS_RESET_STATUS)
>>>                  omap_test_timeout((omap_hwmod_read(oh, sysc->syss_offs)
>>>                                     & SYSS_RESETDONE_MASK),
>>>
>>>
>>> This leads me to believe that the omap_test_timeout functions might
>>> not be working quite right.
>>
>> There may be a srst_udelay needed for some module, see commit
>> ebf244148092 ("ARM: OMAP2+: Use srst_udelay for USB on dm814x")
>> for example.
>>
>> You might be able to find which module it is by commenting out
>> postcore_initcall_sync(omap3_l3_init) in drivers/bus/omap_l3_smx.c
>> temporarily as the system will most likely hang right there.
> 
> I commented out that line as you suggested, but the system boots as
> normal and I get the crash (as normal)
> 
> I am looking through the DM3730 and OMAP3630 TRM now.  Any thought on
> a keyword search I should use to see which hwmods might require
> srst_udelay?

Looking at the log you provided, it looks like only mmc1, mmc3 and i2c1 
are reset during the wlan probe. Based on the prints coming out in the 
failing case, it looks like the culprit might be mmc3 for some reason.

[   18.239746] _wait_softreset_complete: mmc3
[   18.638580] _wait_softreset_complete: mmc3
[   18.657562] _wait_softreset_complete: mmc1
[   18.833374] wlcore: firmware booted (Rev 7.3.10.0.141)

^ the firmware notification above does not come out in the crash.

-Tero

> 
> adam
> 
>>
>> Regards,
>>
>> Tony
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

  reply	other threads:[~2018-01-18  7:34 UTC|newest]

Thread overview: 81+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-11 12:49 [PATCHv2 00/15] clk: ti: cleanups for 4.12 merge window Tero Kristo
2017-03-11 12:49 ` Tero Kristo
2017-03-11 12:49 ` Tero Kristo
2017-03-11 12:49 ` [PATCHv2 01/15] clk: ti: remove un-used definitions from public clk_hw_omap struct Tero Kristo
2017-03-11 12:49   ` Tero Kristo
2017-03-11 12:49   ` Tero Kristo
2017-03-11 12:49 ` [PATCHv2 02/15] clk: ti: add support for automatic clock alias generation Tero Kristo
2017-03-11 12:49   ` Tero Kristo
2017-03-11 12:49   ` Tero Kristo
2017-03-11 12:49 ` [PATCHv2 03/15] clk: ti: add API for creating aliases automatically for simple clock types Tero Kristo
2017-03-11 12:49   ` Tero Kristo
2017-03-11 12:49   ` Tero Kristo
2017-03-11 12:49 ` [PATCHv2 04/15] clk: ti: use automatic clock alias generation framework Tero Kristo
2017-03-11 12:49   ` Tero Kristo
2017-03-11 12:49   ` Tero Kristo
2017-03-11 12:49 ` [PATCHv2 05/15] clk: ti: add clkdm_lookup to the exported functions Tero Kristo
2017-03-11 12:49   ` Tero Kristo
2017-03-11 12:49   ` Tero Kristo
2017-03-11 12:49 ` [PATCHv2 06/15] clk: ti: move omap2_init_clk_clkdm under TI clock driver Tero Kristo
2017-03-11 12:49   ` Tero Kristo
2017-03-11 12:49   ` Tero Kristo
2017-03-11 12:49 ` [PATCHv2 07/15] clk: ti: enforce const types on string arrays Tero Kristo
2017-03-11 12:49   ` Tero Kristo
2017-03-11 12:49   ` Tero Kristo
2017-03-11 12:49 ` [PATCHv2 08/15] clk: ti: omap4: cleanup unnecessary clock aliases Tero Kristo
2017-03-11 12:49   ` Tero Kristo
2017-03-11 12:49   ` Tero Kristo
2017-03-11 12:50 ` [PATCHv2 09/15] clk: ti: drop unnecessary MEMMAP_ADDRESSING flag Tero Kristo
2017-03-11 12:50   ` Tero Kristo
2017-03-11 12:50   ` Tero Kristo
2017-03-11 12:50 ` [PATCHv2 10/15] clk: ti: mux: convert TI mux clock to use its internal data representation Tero Kristo
2017-03-11 12:50   ` Tero Kristo
2017-03-11 12:50   ` Tero Kristo
2017-03-11 12:50 ` [PATCHv2 11/15] clk: ti: divider: convert TI divider clock to use its own " Tero Kristo
2017-03-11 12:50   ` Tero Kristo
2017-03-11 12:50   ` Tero Kristo
2017-03-11 12:50 ` [PATCHv2 12/15] clk: ti: divider: add driver internal API for parsing divider data Tero Kristo
2017-03-11 12:50   ` Tero Kristo
2017-03-11 12:50   ` Tero Kristo
2017-03-11 12:50 ` [PATCHv2 13/15] clk: ti: gate: export gate_clk_ops locally Tero Kristo
2017-03-11 12:50   ` Tero Kristo
2017-03-11 12:50   ` Tero Kristo
2017-03-11 12:50 ` [PATCHv2 14/15] clk: ti: dpll44xx: fix clksel register initialization Tero Kristo
2017-03-11 12:50   ` Tero Kristo
2017-03-11 12:50   ` Tero Kristo
2017-03-11 12:50 ` [PATCHv2 15/15] clk: ti: convert to use proper register definition for all accesses Tero Kristo
2017-03-11 12:50   ` Tero Kristo
2017-03-11 12:50   ` Tero Kristo
2018-01-17 13:27   ` Adam Ford
2018-01-17 13:27     ` Adam Ford
2018-01-17 14:02     ` Tero Kristo
2018-01-17 14:02       ` Tero Kristo
2018-01-17 14:02       ` Tero Kristo
2018-01-17 15:15       ` Adam Ford
2018-01-17 15:15         ` Adam Ford
2018-01-17 15:15         ` Adam Ford
2018-01-17 15:33         ` Adam Ford
2018-01-17 15:33           ` Adam Ford
2018-01-17 21:19         ` Tony Lindgren
2018-01-17 21:19           ` Tony Lindgren
2018-01-17 21:44           ` Adam Ford
2018-01-17 21:44             ` Adam Ford
2018-01-18  7:34             ` Tero Kristo [this message]
2018-01-18  7:34               ` Tero Kristo
2018-01-18  7:34               ` Tero Kristo
2018-01-18 13:26               ` Adam Ford
2018-01-18 13:26                 ` Adam Ford
2018-01-18 13:29                 ` Tero Kristo
2018-01-18 13:29                   ` Tero Kristo
2018-01-18 13:29                   ` Tero Kristo
2018-01-18 14:12                   ` Adam Ford
2018-01-18 14:12                     ` Adam Ford
2018-01-19  9:42                     ` Tero Kristo
2018-01-19  9:42                       ` Tero Kristo
2018-01-19  9:42                       ` Tero Kristo
2018-01-19 16:43                       ` Adam Ford
2018-01-19 16:43                         ` Adam Ford
2018-01-19 16:43                         ` Adam Ford
2018-01-22  7:07                         ` Tero Kristo
2018-01-22  7:07                           ` Tero Kristo
2018-01-22  7:07                           ` Tero Kristo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4f0f185a-2890-3747-7060-1f5b5a77022a@ti.com \
    --to=t-kristo@ti.com \
    --cc=aford173@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=sboyd@codeaurora.org \
    --cc=tony@atomide.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.