Hi Greg,
Today's linux-next merge of the driver-core tree got a conflict in
drivers/net/wireless/ath/ath9k/debug.c between commits 70e535ed0029
("ath9k: clean up debugfs print of reset causes"), 7b8aaead958e
("ath9k: restart hardware after noise floor calibration failure") and
325e18817668 ("ath9k: fix misc debugfs when not using chan context")
from the net-next tree and commit 631bee257bd5 ("ath: use seq_file api
for ath9k debugfs files") from the driver-core tree.
I fixed it up (see below) and can carry the fix as necessary (no action
is required).
Greg, I am not sure why those 2 commits are even in your tree. Do they
depend on something else in your tree?
--
Cheers,
Stephen Rothwell [email protected]
diff --cc drivers/net/wireless/ath/ath9k/debug.c
index 696e3d5309c6,a1f1614a05c2..000000000000
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@@ -832,57 -731,42 +731,46 @@@ static int read_file_misc(struct seq_fi
continue;
ath9k_calculate_iter_data(sc, ctx, &iter_data);
- len += scnprintf(buf + len, sizeof(buf) - len,
- "VIFS: CTX %i(%i) AP: %i STA: %i MESH: %i WDS: %i",
- i++, (int)(ctx->assigned), iter_data.naps,
- iter_data.nstations,
- iter_data.nmeshes, iter_data.nwds);
- len += scnprintf(buf + len, sizeof(buf) - len,
- " ADHOC: %i TOTAL: %hi BEACON-VIF: %hi\n",
- iter_data.nadhocs, sc->cur_chan->nvifs, sc->nbcnvifs);
+ seq_printf(file,
- "VIF-COUNTS: CTX %i AP: %i STA: %i MESH: %i WDS: %i",
- i++, iter_data.naps, iter_data.nstations,
++ "VIFS: CTX %i(%i) AP: %i STA: %i MESH: %i WDS: %i",
++ i++, (int)(ctx->assigned), iter_data.naps,
++ iter_data.nstations,
+ iter_data.nmeshes, iter_data.nwds);
+ seq_printf(file, " ADHOC: %i TOTAL: %hi BEACON-VIF: %hi\n",
+ iter_data.nadhocs, sc->cur_chan->nvifs,
+ sc->nbcnvifs);
}
- if (len > sizeof(buf))
- len = sizeof(buf);
-
- retval = simple_read_from_buffer(user_buf, count, ppos, buf, len);
- return retval;
+ return 0;
}
- static ssize_t read_file_reset(struct file *file, char __user *user_buf,
- size_t count, loff_t *ppos)
+ static int read_file_reset(struct seq_file *file, void *data)
{
- struct ath_softc *sc = file->private_data;
+ struct ath_softc *sc = dev_get_drvdata(file->private);
+ static const char * const reset_cause[__RESET_TYPE_MAX] = {
+ [RESET_TYPE_BB_HANG] = "Baseband Hang",
+ [RESET_TYPE_BB_WATCHDOG] = "Baseband Watchdog",
+ [RESET_TYPE_FATAL_INT] = "Fatal HW Error",
+ [RESET_TYPE_TX_ERROR] = "TX HW error",
+ [RESET_TYPE_TX_GTT] = "Transmit timeout",
+ [RESET_TYPE_TX_HANG] = "TX Path Hang",
+ [RESET_TYPE_PLL_HANG] = "PLL RX Hang",
+ [RESET_TYPE_MAC_HANG] = "MAC Hang",
+ [RESET_TYPE_BEACON_STUCK] = "Stuck Beacon",
+ [RESET_TYPE_MCI] = "MCI Reset",
+ [RESET_TYPE_CALIBRATION] = "Calibration error",
+ };
- char buf[512];
- unsigned int len = 0;
+ int i;
- seq_printf(file, "%17s: %2d\n", "Baseband Hang",
- sc->debug.stats.reset[RESET_TYPE_BB_HANG]);
- seq_printf(file, "%17s: %2d\n", "Baseband Watchdog",
- sc->debug.stats.reset[RESET_TYPE_BB_WATCHDOG]);
- seq_printf(file, "%17s: %2d\n", "Fatal HW Error",
- sc->debug.stats.reset[RESET_TYPE_FATAL_INT]);
- seq_printf(file, "%17s: %2d\n", "TX HW error",
- sc->debug.stats.reset[RESET_TYPE_TX_ERROR]);
- seq_printf(file, "%17s: %2d\n", "TX Path Hang",
- sc->debug.stats.reset[RESET_TYPE_TX_HANG]);
- seq_printf(file, "%17s: %2d\n", "PLL RX Hang",
- sc->debug.stats.reset[RESET_TYPE_PLL_HANG]);
- seq_printf(file, "%17s: %2d\n", "MAC Hang",
- sc->debug.stats.reset[RESET_TYPE_MAC_HANG]);
- seq_printf(file, "%17s: %2d\n", "Stuck Beacon",
- sc->debug.stats.reset[RESET_TYPE_BEACON_STUCK]);
- seq_printf(file, "%17s: %2d\n", "MCI Reset",
- sc->debug.stats.reset[RESET_TYPE_MCI]);
+ for (i = 0; i < ARRAY_SIZE(reset_cause); i++) {
+ if (!reset_cause[i])
+ continue;
+
- len += scnprintf(buf + len, sizeof(buf) - len,
- "%17s: %2d\n", reset_cause[i],
- sc->debug.stats.reset[i]);
++ seq_printf(file, "%17s: %2d\n", reset_cause[i],
++ sc->debug.stats.reset[i]);
+ }
- if (len > sizeof(buf))
- len = sizeof(buf);
-
- return simple_read_from_buffer(user_buf, count, ppos, buf, len);
+ return 0;
}
void ath_debug_stat_tx(struct ath_softc *sc, struct ath_buf *bf,
@@@ -1331,16 -1175,16 +1179,16 @@@ int ath9k_init_debug(struct ath_hw *ah
ath9k_dfs_init_debug(sc);
ath9k_tx99_init_debug(sc);
- ath9k_spectral_init_debug(sc);
+ ath9k_cmn_spectral_init_debug(&sc->spec_priv, sc->debug.debugfs_phy);
- debugfs_create_file("dma", S_IRUSR, sc->debug.debugfs_phy, sc,
- &fops_dma);
- debugfs_create_file("interrupt", S_IRUSR, sc->debug.debugfs_phy, sc,
- &fops_interrupt);
- debugfs_create_file("xmit", S_IRUSR, sc->debug.debugfs_phy, sc,
- &fops_xmit);
- debugfs_create_file("queues", S_IRUSR, sc->debug.debugfs_phy, sc,
- &fops_queues);
+ debugfs_create_devm_seqfile(sc->dev, "dma", sc->debug.debugfs_phy,
+ read_file_dma);
+ debugfs_create_devm_seqfile(sc->dev, "interrupt", sc->debug.debugfs_phy,
+ read_file_interrupt);
+ debugfs_create_devm_seqfile(sc->dev, "xmit", sc->debug.debugfs_phy,
+ read_file_xmit);
+ debugfs_create_devm_seqfile(sc->dev, "queues", sc->debug.debugfs_phy,
+ read_file_queues);
debugfs_create_u32("qlen_bk", S_IRUSR | S_IWUSR, sc->debug.debugfs_phy,
&sc->tx.txq_max_pending[IEEE80211_AC_BK]);
debugfs_create_u32("qlen_be", S_IRUSR | S_IWUSR, sc->debug.debugfs_phy,
On 01-12-14 08:19, Stephen Rothwell wrote:
> Hi Greg,
>
> Today's linux-next merge of the driver-core tree got a conflict in
> drivers/net/wireless/ath/ath9k/debug.c between commits 70e535ed0029
> ("ath9k: clean up debugfs print of reset causes"), 7b8aaead958e
> ("ath9k: restart hardware after noise floor calibration failure") and
> 325e18817668 ("ath9k: fix misc debugfs when not using chan context")
> from the net-next tree and commit 631bee257bd5 ("ath: use seq_file api
> for ath9k debugfs files") from the driver-core tree.
>
> I fixed it up (see below) and can carry the fix as necessary (no action
> is required).
>
> Greg, I am not sure why those 2 commits are even in your tree. Do they
> depend on something else in your tree?
They do. The three commits below are related:
d32394f ath: ath9k: use debugfs_create_devm_seqfile() helper for
seq_file entrie
631bee2 ath: use seq_file api for ath9k debugfs files
98210b7 debugfs: add helper function to create device related seq_file
The ath patches were made to provide example of using the new helper
function and get some idea about code savings. Greg and John discussed
who would take them. I noticed other ath changes in net-next so I kinda
expected this email ;-)
Regards,
Arend
all,
On Mon, Dec 01, 2014 at 08:34:05AM +0100, Arend van Spriel wrote:
> On 01-12-14 08:19, Stephen Rothwell wrote:
> > Hi Greg,
> >
> > Today's linux-next merge of the driver-core tree got a conflict in
> > drivers/net/wireless/ath/ath9k/debug.c between commits 70e535ed0029
> > ("ath9k: clean up debugfs print of reset causes"), 7b8aaead958e
> > ("ath9k: restart hardware after noise floor calibration failure") and
> > 325e18817668 ("ath9k: fix misc debugfs when not using chan context")
> > from the net-next tree and commit 631bee257bd5 ("ath: use seq_file api
> > for ath9k debugfs files") from the driver-core tree.
> >
> > I fixed it up (see below) and can carry the fix as necessary (no action
> > is required).
> >
> > Greg, I am not sure why those 2 commits are even in your tree. Do they
> > depend on something else in your tree?
>
> They do. The three commits below are related:
>
> d32394f ath: ath9k: use debugfs_create_devm_seqfile() helper for
> seq_file entrie
> 631bee2 ath: use seq_file api for ath9k debugfs files
> 98210b7 debugfs: add helper function to create device related seq_file
>
> The ath patches were made to provide example of using the new helper
> function and get some idea about code savings. Greg and John discussed
> who would take them. I noticed other ath changes in net-next so I kinda
> expected this email ;-)
>
> Regards,
> Arend
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
I just ran in to a problem with one of these commits.
On an Acer C720 laptop if a suspend is performed the screen freezes,
the machine locks up, and according to the indicator lights it does
not enter suspend. A hard reset is required to get it running again.
I have bisected the kernel and found that the following is the first bad
commit.
commit d32394fae95741d733b174ec1446f27765f80233
Author: Arend van Spriel <[email protected]>
Date: Sun Nov 9 11:32:00 2014 +0100
ath: ath9k: use debugfs_create_devm_seqfile() helper for seq_file
entries
Use the helper to get rid of the file operations per debugfs file.
The
struct ath9k_softc pointer is set as device driver data to be
obtained
in the seq_file read operation.
Signed-off-by: Arend van Spriel <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
Let me know if I can do anything else to help.
--
- Jeremiah Mahler
On Wed, Dec 03, 2014 at 12:36:55AM -0800, Jeremiah Mahler wrote:
> all,
>
> On Mon, Dec 01, 2014 at 08:34:05AM +0100, Arend van Spriel wrote:
> > On 01-12-14 08:19, Stephen Rothwell wrote:
> > > Hi Greg,
> > >
> > > Today's linux-next merge of the driver-core tree got a conflict in
> > > drivers/net/wireless/ath/ath9k/debug.c between commits 70e535ed0029
> > > ("ath9k: clean up debugfs print of reset causes"), 7b8aaead958e
> > > ("ath9k: restart hardware after noise floor calibration failure") and
> > > 325e18817668 ("ath9k: fix misc debugfs when not using chan context")
> > > from the net-next tree and commit 631bee257bd5 ("ath: use seq_file api
> > > for ath9k debugfs files") from the driver-core tree.
> > >
> > > I fixed it up (see below) and can carry the fix as necessary (no action
> > > is required).
> > >
> > > Greg, I am not sure why those 2 commits are even in your tree. Do they
> > > depend on something else in your tree?
> >
> > They do. The three commits below are related:
> >
> > d32394f ath: ath9k: use debugfs_create_devm_seqfile() helper for
> > seq_file entrie
> > 631bee2 ath: use seq_file api for ath9k debugfs files
> > 98210b7 debugfs: add helper function to create device related seq_file
> >
> > The ath patches were made to provide example of using the new helper
> > function and get some idea about code savings. Greg and John discussed
> > who would take them. I noticed other ath changes in net-next so I kinda
> > expected this email ;-)
> >
> > Regards,
> > Arend
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
>
> I just ran in to a problem with one of these commits.
>
> On an Acer C720 laptop if a suspend is performed the screen freezes,
> the machine locks up, and according to the indicator lights it does
> not enter suspend. A hard reset is required to get it running again.
>
> I have bisected the kernel and found that the following is the first bad
> commit.
>
> commit d32394fae95741d733b174ec1446f27765f80233
> Author: Arend van Spriel <[email protected]>
> Date: Sun Nov 9 11:32:00 2014 +0100
>
> ath: ath9k: use debugfs_create_devm_seqfile() helper for seq_file
> entries
>
> Use the helper to get rid of the file operations per debugfs file.
> The
> struct ath9k_softc pointer is set as device driver data to be
> obtained
> in the seq_file read operation.
>
> Signed-off-by: Arend van Spriel <[email protected]>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
>
> Let me know if I can do anything else to help.
>
> --
> - Jeremiah Mahler
I took a look at the patch that is causing this problem (d32394fae95).
My config negates everything in the patch except for a one line change
to ath9k/pci.c. If I remove this change (shown below) the problem goes
away.
diff --git a/drivers/net/wireless/ath/ath9k/pci.c
b/drivers/net/wireless/ath/ath9k/pci.c
index 90c9e3c..c018dea 100644
--- a/drivers/net/wireless/ath/ath9k/pci.c
+++ b/drivers/net/wireless/ath/ath9k/pci.c
@@ -856,7 +856,6 @@ static int ath_pci_probe(struct pci_dev *pdev, const
struct pci_device_id *id)
sc = hw->priv;
sc->hw = hw;
sc->dev = &pdev->dev;
- dev_set_drvdata(sc->dev, sc);
sc->mem = pcim_iomap_table(pdev)[0];
sc->driver_data = id->driver_data;
--
- Jeremiah Mahler
On 12/03/14 11:51, Jeremiah Mahler wrote:
> On Wed, Dec 03, 2014 at 12:36:55AM -0800, Jeremiah Mahler wrote:
>> all,
>>
>> On Mon, Dec 01, 2014 at 08:34:05AM +0100, Arend van Spriel wrote:
>>> On 01-12-14 08:19, Stephen Rothwell wrote:
>>>> Hi Greg,
>>>>
>>>> Today's linux-next merge of the driver-core tree got a conflict in
>>>> drivers/net/wireless/ath/ath9k/debug.c between commits 70e535ed0029
>>>> ("ath9k: clean up debugfs print of reset causes"), 7b8aaead958e
>>>> ("ath9k: restart hardware after noise floor calibration failure") and
>>>> 325e18817668 ("ath9k: fix misc debugfs when not using chan context")
>>>> from the net-next tree and commit 631bee257bd5 ("ath: use seq_file api
>>>> for ath9k debugfs files") from the driver-core tree.
>>>>
>>>> I fixed it up (see below) and can carry the fix as necessary (no action
>>>> is required).
>>>>
>>>> Greg, I am not sure why those 2 commits are even in your tree. Do they
>>>> depend on something else in your tree?
>>>
>>> They do. The three commits below are related:
>>>
>>> d32394f ath: ath9k: use debugfs_create_devm_seqfile() helper for
>>> seq_file entrie
>>> 631bee2 ath: use seq_file api for ath9k debugfs files
>>> 98210b7 debugfs: add helper function to create device related seq_file
>>>
>>> The ath patches were made to provide example of using the new helper
>>> function and get some idea about code savings. Greg and John discussed
>>> who would take them. I noticed other ath changes in net-next so I kinda
>>> expected this email ;-)
>>>
>>> Regards,
>>> Arend
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at http://www.tux.org/lkml/
>>
>> I just ran in to a problem with one of these commits.
>>
>> On an Acer C720 laptop if a suspend is performed the screen freezes,
>> the machine locks up, and according to the indicator lights it does
>> not enter suspend. A hard reset is required to get it running again.
>>
>> I have bisected the kernel and found that the following is the first bad
>> commit.
>>
>> commit d32394fae95741d733b174ec1446f27765f80233
>> Author: Arend van Spriel<[email protected]>
>> Date: Sun Nov 9 11:32:00 2014 +0100
>>
>> ath: ath9k: use debugfs_create_devm_seqfile() helper for seq_file
>> entries
>>
>> Use the helper to get rid of the file operations per debugfs file.
>> The
>> struct ath9k_softc pointer is set as device driver data to be
>> obtained
>> in the seq_file read operation.
>>
>> Signed-off-by: Arend van Spriel<[email protected]>
>> Signed-off-by: Greg Kroah-Hartman<[email protected]>
>>
>> Let me know if I can do anything else to help.
>>
>> --
>> - Jeremiah Mahler
>
> I took a look at the patch that is causing this problem (d32394fae95).
> My config negates everything in the patch except for a one line change
> to ath9k/pci.c. If I remove this change (shown below) the problem goes
> away.
Ok. But then it will likely crash when you cat one of the changed
debugfs files. Guess this commit needs to be reverted entirely.
Regards,
Arend
> diff --git a/drivers/net/wireless/ath/ath9k/pci.c
> b/drivers/net/wireless/ath/ath9k/pci.c
> index 90c9e3c..c018dea 100644
> --- a/drivers/net/wireless/ath/ath9k/pci.c
> +++ b/drivers/net/wireless/ath/ath9k/pci.c
> @@ -856,7 +856,6 @@ static int ath_pci_probe(struct pci_dev *pdev, const
> struct pci_device_id *id)
> sc = hw->priv;
> sc->hw = hw;
> sc->dev =&pdev->dev;
> - dev_set_drvdata(sc->dev, sc);
> sc->mem = pcim_iomap_table(pdev)[0];
> sc->driver_data = id->driver_data;
>
On Wed, Dec 03, 2014 at 01:49:00PM +0100, Arend van Spriel wrote:
> On 12/03/14 11:51, Jeremiah Mahler wrote:
> >On Wed, Dec 03, 2014 at 12:36:55AM -0800, Jeremiah Mahler wrote:
> >>all,
> >>
> >>On Mon, Dec 01, 2014 at 08:34:05AM +0100, Arend van Spriel wrote:
> >>>On 01-12-14 08:19, Stephen Rothwell wrote:
> >>>>Hi Greg,
> >>>>
> >>>>Today's linux-next merge of the driver-core tree got a conflict in
> >>>>drivers/net/wireless/ath/ath9k/debug.c between commits 70e535ed0029
> >>>>("ath9k: clean up debugfs print of reset causes"), 7b8aaead958e
> >>>>("ath9k: restart hardware after noise floor calibration failure") and
> >>>>325e18817668 ("ath9k: fix misc debugfs when not using chan context")
> >>>>from the net-next tree and commit 631bee257bd5 ("ath: use seq_file api
> >>>>for ath9k debugfs files") from the driver-core tree.
> >>>>
> >>>>I fixed it up (see below) and can carry the fix as necessary (no action
> >>>>is required).
> >>>>
> >>>>Greg, I am not sure why those 2 commits are even in your tree. Do they
> >>>>depend on something else in your tree?
> >>>
> >>>They do. The three commits below are related:
> >>>
> >>>d32394f ath: ath9k: use debugfs_create_devm_seqfile() helper for
> >>>seq_file entrie
> >>>631bee2 ath: use seq_file api for ath9k debugfs files
> >>>98210b7 debugfs: add helper function to create device related seq_file
> >>>
> >>>The ath patches were made to provide example of using the new helper
> >>>function and get some idea about code savings. Greg and John discussed
> >>>who would take them. I noticed other ath changes in net-next so I kinda
> >>>expected this email ;-)
> >>>
> >>>Regards,
> >>>Arend
> >>>--
> >>>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> >>>the body of a message to [email protected]
> >>>More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>>Please read the FAQ at http://www.tux.org/lkml/
> >>
> >>I just ran in to a problem with one of these commits.
> >>
> >>On an Acer C720 laptop if a suspend is performed the screen freezes,
> >>the machine locks up, and according to the indicator lights it does
> >>not enter suspend. A hard reset is required to get it running again.
> >>
> >>I have bisected the kernel and found that the following is the first bad
> >>commit.
> >>
> >> commit d32394fae95741d733b174ec1446f27765f80233
> >> Author: Arend van Spriel<[email protected]>
> >> Date: Sun Nov 9 11:32:00 2014 +0100
> >>
> >> ath: ath9k: use debugfs_create_devm_seqfile() helper for seq_file
> >> entries
> >>
> >> Use the helper to get rid of the file operations per debugfs file.
> >> The
> >> struct ath9k_softc pointer is set as device driver data to be
> >> obtained
> >> in the seq_file read operation.
> >>
> >> Signed-off-by: Arend van Spriel<[email protected]>
> >> Signed-off-by: Greg Kroah-Hartman<[email protected]>
> >>
> >>Let me know if I can do anything else to help.
> >>
> >>--
> >>- Jeremiah Mahler
> >
> >I took a look at the patch that is causing this problem (d32394fae95).
> >My config negates everything in the patch except for a one line change
> >to ath9k/pci.c. If I remove this change (shown below) the problem goes
> >away.
>
> Ok. But then it will likely crash when you cat one of the changed debugfs
> files. Guess this commit needs to be reverted entirely.
What commit, d32394fae95741d733b174ec1446f27765f80233?
On 12/03/14 17:21, Greg KH wrote:
> On Wed, Dec 03, 2014 at 01:49:00PM +0100, Arend van Spriel wrote:
>> On 12/03/14 11:51, Jeremiah Mahler wrote:
>>> On Wed, Dec 03, 2014 at 12:36:55AM -0800, Jeremiah Mahler wrote:
>>>> all,
>>>>
>>>> On Mon, Dec 01, 2014 at 08:34:05AM +0100, Arend van Spriel wrote:
>>>>> On 01-12-14 08:19, Stephen Rothwell wrote:
>>>>>> Hi Greg,
>>>>>>
>>>>>> Today's linux-next merge of the driver-core tree got a conflict in
>>>>>> drivers/net/wireless/ath/ath9k/debug.c between commits 70e535ed0029
>>>>>> ("ath9k: clean up debugfs print of reset causes"), 7b8aaead958e
>>>>>> ("ath9k: restart hardware after noise floor calibration failure") and
>>>>>> 325e18817668 ("ath9k: fix misc debugfs when not using chan context")
>>>>> >from the net-next tree and commit 631bee257bd5 ("ath: use seq_file api
>>>>>> for ath9k debugfs files") from the driver-core tree.
>>>>>>
>>>>>> I fixed it up (see below) and can carry the fix as necessary (no action
>>>>>> is required).
>>>>>>
>>>>>> Greg, I am not sure why those 2 commits are even in your tree. Do they
>>>>>> depend on something else in your tree?
>>>>>
>>>>> They do. The three commits below are related:
>>>>>
>>>>> d32394f ath: ath9k: use debugfs_create_devm_seqfile() helper for
>>>>> seq_file entrie
>>>>> 631bee2 ath: use seq_file api for ath9k debugfs files
>>>>> 98210b7 debugfs: add helper function to create device related seq_file
>>>>>
>>>>> The ath patches were made to provide example of using the new helper
>>>>> function and get some idea about code savings. Greg and John discussed
>>>>> who would take them. I noticed other ath changes in net-next so I kinda
>>>>> expected this email ;-)
>>>>>
>>>>> Regards,
>>>>> Arend
>>>>> --
>>>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>>>> the body of a message to [email protected]
>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>>> Please read the FAQ at http://www.tux.org/lkml/
>>>>
>>>> I just ran in to a problem with one of these commits.
>>>>
>>>> On an Acer C720 laptop if a suspend is performed the screen freezes,
>>>> the machine locks up, and according to the indicator lights it does
>>>> not enter suspend. A hard reset is required to get it running again.
>>>>
>>>> I have bisected the kernel and found that the following is the first bad
>>>> commit.
>>>>
>>>> commit d32394fae95741d733b174ec1446f27765f80233
>>>> Author: Arend van Spriel<[email protected]>
>>>> Date: Sun Nov 9 11:32:00 2014 +0100
>>>>
>>>> ath: ath9k: use debugfs_create_devm_seqfile() helper for seq_file
>>>> entries
>>>>
>>>> Use the helper to get rid of the file operations per debugfs file.
>>>> The
>>>> struct ath9k_softc pointer is set as device driver data to be
>>>> obtained
>>>> in the seq_file read operation.
>>>>
>>>> Signed-off-by: Arend van Spriel<[email protected]>
>>>> Signed-off-by: Greg Kroah-Hartman<[email protected]>
>>>>
>>>> Let me know if I can do anything else to help.
>>>>
>>>> --
>>>> - Jeremiah Mahler
>>>
>>> I took a look at the patch that is causing this problem (d32394fae95).
>>> My config negates everything in the patch except for a one line change
>>> to ath9k/pci.c. If I remove this change (shown below) the problem goes
>>> away.
>>
>> Ok. But then it will likely crash when you cat one of the changed debugfs
>> files. Guess this commit needs to be reverted entirely.
>
> What commit, d32394fae95741d733b174ec1446f27765f80233?
Indeed.
Regards,
Arend
Arend,
On Wed, Dec 03, 2014 at 01:49:00PM +0100, Arend van Spriel wrote:
> On 12/03/14 11:51, Jeremiah Mahler wrote:
> >On Wed, Dec 03, 2014 at 12:36:55AM -0800, Jeremiah Mahler wrote:
> >>all,
> >>
> >>On Mon, Dec 01, 2014 at 08:34:05AM +0100, Arend van Spriel wrote:
> >>>On 01-12-14 08:19, Stephen Rothwell wrote:
> >>>>Hi Greg,
> >>>>
> >>>>Today's linux-next merge of the driver-core tree got a conflict in
> >>>>drivers/net/wireless/ath/ath9k/debug.c between commits 70e535ed0029
> >>>>("ath9k: clean up debugfs print of reset causes"), 7b8aaead958e
> >>>>("ath9k: restart hardware after noise floor calibration failure") and
> >>>>325e18817668 ("ath9k: fix misc debugfs when not using chan context")
> >>>>from the net-next tree and commit 631bee257bd5 ("ath: use seq_file api
> >>>>for ath9k debugfs files") from the driver-core tree.
> >>>>
> >>>>I fixed it up (see below) and can carry the fix as necessary (no action
> >>>>is required).
> >>>>
> >>>>Greg, I am not sure why those 2 commits are even in your tree. Do they
> >>>>depend on something else in your tree?
> >>>
> >>>They do. The three commits below are related:
> >>>
> >>>d32394f ath: ath9k: use debugfs_create_devm_seqfile() helper for
> >>>seq_file entrie
> >>>631bee2 ath: use seq_file api for ath9k debugfs files
> >>>98210b7 debugfs: add helper function to create device related seq_file
> >>>
> >>>The ath patches were made to provide example of using the new helper
> >>>function and get some idea about code savings. Greg and John discussed
> >>>who would take them. I noticed other ath changes in net-next so I kinda
> >>>expected this email ;-)
> >>>
> >>>Regards,
> >>>Arend
> >>>--
> >>>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> >>>the body of a message to [email protected]
> >>>More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>>Please read the FAQ at http://www.tux.org/lkml/
> >>
> >>I just ran in to a problem with one of these commits.
> >>
> >>On an Acer C720 laptop if a suspend is performed the screen freezes,
> >>the machine locks up, and according to the indicator lights it does
> >>not enter suspend. A hard reset is required to get it running again.
> >>
> >>I have bisected the kernel and found that the following is the first bad
> >>commit.
> >>
> >> commit d32394fae95741d733b174ec1446f27765f80233
> >> Author: Arend van Spriel<[email protected]>
> >> Date: Sun Nov 9 11:32:00 2014 +0100
> >>
> >> ath: ath9k: use debugfs_create_devm_seqfile() helper for seq_file
> >> entries
> >>
> >> Use the helper to get rid of the file operations per debugfs file.
> >> The
> >> struct ath9k_softc pointer is set as device driver data to be
> >> obtained
> >> in the seq_file read operation.
> >>
> >> Signed-off-by: Arend van Spriel<[email protected]>
> >> Signed-off-by: Greg Kroah-Hartman<[email protected]>
> >>
> >>Let me know if I can do anything else to help.
> >>
> >>--
> >>- Jeremiah Mahler
> >
> >I took a look at the patch that is causing this problem (d32394fae95).
> >My config negates everything in the patch except for a one line change
> >to ath9k/pci.c. If I remove this change (shown below) the problem goes
> >away.
>
> Ok. But then it will likely crash when you cat one of the changed debugfs
> files. Guess this commit needs to be reverted entirely.
>
On my machine ATH9K_DEBUFS is disabled so none of that code is used.
I can try enabling it and see if a cat will crash it.
> Regards,
> Arend
>
> >diff --git a/drivers/net/wireless/ath/ath9k/pci.c
> >b/drivers/net/wireless/ath/ath9k/pci.c
> >index 90c9e3c..c018dea 100644
> >--- a/drivers/net/wireless/ath/ath9k/pci.c
> >+++ b/drivers/net/wireless/ath/ath9k/pci.c
> >@@ -856,7 +856,6 @@ static int ath_pci_probe(struct pci_dev *pdev, const
> >struct pci_device_id *id)
> > sc = hw->priv;
> > sc->hw = hw;
> > sc->dev =&pdev->dev;
> >- dev_set_drvdata(sc->dev, sc);
> > sc->mem = pcim_iomap_table(pdev)[0];
> > sc->driver_data = id->driver_data;
> >
>
--
- Jeremiah Mahler
On Wed, Dec 03, 2014 at 01:06:59PM -0800, Jeremiah Mahler wrote:
> Arend,
>
> On Wed, Dec 03, 2014 at 01:49:00PM +0100, Arend van Spriel wrote:
> > On 12/03/14 11:51, Jeremiah Mahler wrote:
> > >On Wed, Dec 03, 2014 at 12:36:55AM -0800, Jeremiah Mahler wrote:
> > >>all,
> > >>
> > >>On Mon, Dec 01, 2014 at 08:34:05AM +0100, Arend van Spriel wrote:
> > >>>On 01-12-14 08:19, Stephen Rothwell wrote:
> > >>>>Hi Greg,
> > >>>>
[...]
> > >
> > >I took a look at the patch that is causing this problem (d32394fae95).
> > >My config negates everything in the patch except for a one line change
> > >to ath9k/pci.c. If I remove this change (shown below) the problem goes
> > >away.
> >
> > Ok. But then it will likely crash when you cat one of the changed debugfs
> > files. Guess this commit needs to be reverted entirely.
> >
>
> On my machine ATH9K_DEBUFS is disabled so none of that code is used.
>
> I can try enabling it and see if a cat will crash it.
>
With ATH9K_DEBUGFS enabled, cat works fine on the files that were
changed.
/sys/kernel/debug/ieee80211/phy0/ath9k/
dma
interrupt
xmit
queues
> > Regards,
> > Arend
> >
[...]
>
> --
> - Jeremiah Mahler
--
- Jeremiah Mahler
Arend,
I haven't heard if you have looked at this bug at all yet. I was
curious so I looked at it some more and I have some more information
that might be helpful.
On Wed, Dec 03, 2014 at 01:41:28PM -0800, Jeremiah Mahler wrote:
> On Wed, Dec 03, 2014 at 01:06:59PM -0800, Jeremiah Mahler wrote:
> > Arend,
> >
[...]
> > > >
> > > >I took a look at the patch that is causing this problem (d32394fae95).
> > > >My config negates everything in the patch except for a one line change
> > > >to ath9k/pci.c. If I remove this change (shown below) the problem goes
> > > >away.
> > >
> > > Ok. But then it will likely crash when you cat one of the changed debugfs
> > > files. Guess this commit needs to be reverted entirely.
> > >
[...]
Referring to the code snippet below, notice that both pci_set_drvdata()
and dev_set_drvdata() are called. If the call to dev_set_drvdata() is
removed the bug goes away.
drivers/net/wireless/ath/ath9k/pci.c
861 SET_IEEE80211_DEV(hw, &pdev->dev);
862 pci_set_drvdata(pdev, hw);
863
864 sc = hw->priv;
865 sc->hw = hw;
866 sc->dev = &pdev->dev;
867 dev_set_drvdata(sc->dev, sc);
868 sc->mem = pcim_iomap_table(pdev)[0];
869 sc->driver_data = id->driver_data;
Translating the call to pci_set_drvdata() produces the following.
pci_set_drvdata(pdev, hw);
(translating from include/linux/pci.h)
dev_set_drvdata(&pdev->dev, hw)
(translating from include/linux/device.h)
&pdev->dev->driver = hw;
And doing the same for dev_set_drvdata().
dev_set_drvdata(sc->dev, sc)
(sc->dev = &pdev->dev)
dev_set_drvdata(&pdev->dev, sc)
(translating from include/linux/device.h)
&pdev->dev->driver = sc;
The same destination is being set with two different values. Then calls
to pci_get_drvdata(), which expect hw, are getting sc, and it breaks.
--
- Jeremiah Mahler
On 04-12-14 11:19, Jeremiah Mahler wrote:
> Arend,
>
> I haven't heard if you have looked at this bug at all yet. I was
> curious so I looked at it some more and I have some more information
> that might be helpful.
>
> On Wed, Dec 03, 2014 at 01:41:28PM -0800, Jeremiah Mahler wrote:
>> On Wed, Dec 03, 2014 at 01:06:59PM -0800, Jeremiah Mahler wrote:
>>> Arend,
>>>
> [...]
>>>>>
>>>>> I took a look at the patch that is causing this problem (d32394fae95).
>>>>> My config negates everything in the patch except for a one line change
>>>>> to ath9k/pci.c. If I remove this change (shown below) the problem goes
>>>>> away.
>>>>
>>>> Ok. But then it will likely crash when you cat one of the changed debugfs
>>>> files. Guess this commit needs to be reverted entirely.
>>>>
> [...]
>
> Referring to the code snippet below, notice that both pci_set_drvdata()
> and dev_set_drvdata() are called. If the call to dev_set_drvdata() is
> removed the bug goes away.
>
> drivers/net/wireless/ath/ath9k/pci.c
>
> 861 SET_IEEE80211_DEV(hw, &pdev->dev);
> 862 pci_set_drvdata(pdev, hw);
> 863
> 864 sc = hw->priv;
> 865 sc->hw = hw;
> 866 sc->dev = &pdev->dev;
> 867 dev_set_drvdata(sc->dev, sc);
> 868 sc->mem = pcim_iomap_table(pdev)[0];
> 869 sc->driver_data = id->driver_data;
>
> Translating the call to pci_set_drvdata() produces the following.
>
> pci_set_drvdata(pdev, hw);
> (translating from include/linux/pci.h)
> dev_set_drvdata(&pdev->dev, hw)
> (translating from include/linux/device.h)
> &pdev->dev->driver = hw;
>
> And doing the same for dev_set_drvdata().
>
> dev_set_drvdata(sc->dev, sc)
> (sc->dev = &pdev->dev)
> dev_set_drvdata(&pdev->dev, sc)
> (translating from include/linux/device.h)
> &pdev->dev->driver = sc;
>
> The same destination is being set with two different values. Then calls
> to pci_get_drvdata(), which expect hw, are getting sc, and it breaks.
Yes, that was my suspicion as well. This is why I asked to revert the
patch entirely. I see that sc equals hw->priv so I know how to correct
it. However, with merge window around the corner it may be easier to do
a revert.
Regards,
Arend