Search Linux Wireless

Re: [PATCH v6 9/9] ath5k: Fix reset and interrupts for AHB type of devices.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 2010/11/26 Wojciech Dubowik <dubowoj@xxxxxxxxxxx>:
> > From: Felix Fietkau <nbd@xxxxxxxxxxx>
> >
> > On WiSoc we cannot access mac register before it is resetted.
> > It will crash hardware otherwise.
> >
> > Signed-off-by: Felix Fietkau <nbd@xxxxxxxxxxx>
> > Signed-off-by: Wojciech Dubowik <Wojciech.Dubowik@xxxxxxxxxxx>
> > ---
> > Âdrivers/net/wireless/ath/ath5k/base.c Â| Â Â7 ++-
> > Âdrivers/net/wireless/ath/ath5k/reset.c | Â114
> ++++++++++++++++++++++++-------
> > Â2 files changed, 94 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/net/wireless/ath/ath5k/base.c
> b/drivers/net/wireless/ath/ath5k/base.c
> > index 4d5ac71..87a4bb6 100644
> > --- a/drivers/net/wireless/ath/ath5k/base.c
> > +++ b/drivers/net/wireless/ath/ath5k/base.c
> > @@ -2169,7 +2169,8 @@ ath5k_intr(int irq, void *dev_id)
> > Â Â Â Âunsigned int counter = 1000;
> >
> > Â Â Â Âif (unlikely(test_bit(ATH_STAT_INVALID, sc->status) ||
> > - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â !ath5k_hw_is_intr_pending(ah)))
> > + Â Â Â Â Â Â Â ((ath5k_get_bus_type(ah) != ATH_AHB) &&
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â !ath5k_hw_is_intr_pending(ah))))
> > Â Â Â Â Â Â Â Âreturn IRQ_NONE;
> >
> > Â Â Â Âdo {
> > @@ -2235,6 +2236,10 @@ ath5k_intr(int irq, void *dev_id)
> > Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â
> Âtasklet_schedule(&sc->rf_kill.toggleq);
> >
> > Â Â Â Â Â Â Â Â}
> > +
> > + Â Â Â Â Â Â Â if (ath5k_get_bus_type(ah) == ATH_AHB)
> > + Â Â Â Â Â Â Â Â Â Â Â break;
> > +
> > Â Â Â Â} while (ath5k_hw_is_intr_pending(ah) && --counter > 0);
> >
> > Â Â Â Âif (unlikely(!counter))
> > diff --git a/drivers/net/wireless/ath/ath5k/reset.c
> b/drivers/net/wireless/ath/ath5k/reset.c
> > index 198a146..4f54655 100644
> > --- a/drivers/net/wireless/ath/ath5k/reset.c
> > +++ b/drivers/net/wireless/ath/ath5k/reset.c
> > @@ -27,6 +27,7 @@
> >
> > Â#include <linux/pci.h> Â Â Â Â Â Â Â Â /* To determine if a card is
> pci-e */
> > Â#include <linux/log2.h>
> > +#include <linux/platform_device.h>
> > Â#include "ath5k.h"
> > Â#include "reg.h"
> > Â#include "base.h"
> > @@ -198,31 +199,74 @@ static inline void
> ath5k_hw_write_rate_duration(struct ath5k_hw *ah,
> > Â*/
> > Âstatic int ath5k_hw_nic_reset(struct ath5k_hw *ah, u32 val)
> > Â{
> > - Â Â Â int ret;
> > + Â Â Â int ret = 0;
> > Â Â Â Âu32 mask = val ? val : ~0U;
> >
> > Â Â Â Â/* Read-and-clear RX Descriptor Pointer*/
> > - Â Â Â ath5k_hw_reg_read(ah, AR5K_RXDP);
> > + Â Â Â if (!(mask & AR5K_RESET_CTL_MAC))
> > + Â Â Â Â Â Â Â ath5k_hw_reg_read(ah, AR5K_RXDP);
> >
> > Â Â Â Â/*
> > Â Â Â Â * Reset the device and wait until success
> > Â Â Â Â */
> > - Â Â Â ath5k_hw_reg_write(ah, val, AR5K_RESET_CTL);
> > + Â Â Â if (ath5k_get_bus_type(ah) == ATH_AHB) {
> > + Â Â Â Â Â Â Â volatile u32 *reg;
> > + Â Â Â Â Â Â Â u32 regval;
> > + Â Â Â Â Â Â Â val = 0;
> > +
> > + Â Â Â Â Â Â Â /* ah->ah_mac_srev is not available at this point
> yet */
> > + Â Â Â Â Â Â Â if (ah->ah_sc->devid >= AR5K_SREV_AR2315_R6) {
> > + Â Â Â Â Â Â Â Â Â Â Â reg = (u32 *) AR5K_AR2315_RESET;
> > + Â Â Â Â Â Â Â Â Â Â Â if (mask & AR5K_RESET_CTL_MAC)
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â val |= AR5K_AR2315_RESET_WMAC;
> > + Â Â Â Â Â Â Â Â Â Â Â if (mask & AR5K_RESET_CTL_BASEBAND)
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â val |= AR5K_AR2315_RESET_BB_WARM;
> > + Â Â Â Â Â Â Â } else {
> > + Â Â Â Â Â Â Â Â Â Â Â reg = (u32 *) AR5K_AR5312_RESET;
> > + Â Â Â Â Â Â Â Â Â Â Â if (to_platform_device(ah->ah_sc->dev)->id
> == 0) {
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (mask & AR5K_RESET_CTL_MAC)
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â val |=
> AR5K_AR5312_RESET_WMAC0;
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (mask & AR5K_RESET_CTL_BASEBAND)
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â val |=
> AR5K_AR5312_RESET_BB0_COLD |
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â
> ÂAR5K_AR5312_RESET_BB0_WARM;
> > + Â Â Â Â Â Â Â Â Â Â Â } else {
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (mask & AR5K_RESET_CTL_MAC)
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â val |=
> AR5K_AR5312_RESET_WMAC1;
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (mask & AR5K_RESET_CTL_BASEBAND)
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â val |=
> AR5K_AR5312_RESET_BB1_COLD |
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â
> ÂAR5K_AR5312_RESET_BB1_WARM;
> > + Â Â Â Â Â Â Â Â Â Â Â }
> > + Â Â Â Â Â Â Â }
> >
> > - Â Â Â /* Wait at least 128 PCI clocks */
> > - Â Â Â udelay(15);
> > + Â Â Â Â Â Â Â /* Put BB/MAC into reset */
> > + Â Â Â Â Â Â Â regval = __raw_readl(reg);
> > + Â Â Â Â Â Â Â __raw_writel(regval | val, reg);
> > + Â Â Â Â Â Â Â regval = __raw_readl(reg);
> > + Â Â Â Â Â Â Â udelay(100);
> >
> > - Â Â Â if (ah->ah_version == AR5K_AR5210) {
> > - Â Â Â Â Â Â Â val &= AR5K_RESET_CTL_PCU | AR5K_RESET_CTL_DMA
> > - Â Â Â Â Â Â Â Â Â Â Â | AR5K_RESET_CTL_MAC | AR5K_RESET_CTL_PHY;
> > - Â Â Â Â Â Â Â mask &= AR5K_RESET_CTL_PCU | AR5K_RESET_CTL_DMA
> > - Â Â Â Â Â Â Â Â Â Â Â | AR5K_RESET_CTL_MAC | AR5K_RESET_CTL_PHY;
> > + Â Â Â Â Â Â Â /* Bring BB/MAC out of reset */
> > + Â Â Â Â Â Â Â __raw_writel(regval & ~val, reg);
> > + Â Â Â Â Â Â Â regval = __raw_readl(reg);
> > Â Â Â Â} else {
> > - Â Â Â Â Â Â Â val &= AR5K_RESET_CTL_PCU | AR5K_RESET_CTL_BASEBAND;
> > - Â Â Â Â Â Â Â mask &= AR5K_RESET_CTL_PCU |
> AR5K_RESET_CTL_BASEBAND;
> > - Â Â Â }
> >
> > - Â Â Â ret = ath5k_hw_register_timeout(ah, AR5K_RESET_CTL, mask,
> val, false);
> > + Â Â Â Â Â Â Â ath5k_hw_reg_write(ah, val, AR5K_RESET_CTL);
> > +
> > + Â Â Â Â Â Â Â /* Wait at least 128 PCI clocks */
> > + Â Â Â Â Â Â Â udelay(15);
> > +
> > + Â Â Â Â Â Â Â if (ah->ah_version == AR5K_AR5210) {
> > + Â Â Â Â Â Â Â Â Â Â Â val &= AR5K_RESET_CTL_PCU |
> AR5K_RESET_CTL_DMA
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | AR5K_RESET_CTL_MAC |
> AR5K_RESET_CTL_PHY;
> > + Â Â Â Â Â Â Â Â Â Â Â mask &= AR5K_RESET_CTL_PCU |
> AR5K_RESET_CTL_DMA
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | AR5K_RESET_CTL_MAC |
> AR5K_RESET_CTL_PHY;
> > + Â Â Â Â Â Â Â } else {
> > + Â Â Â Â Â Â Â Â Â Â Â val &= AR5K_RESET_CTL_PCU |
> AR5K_RESET_CTL_BASEBAND;
> > + Â Â Â Â Â Â Â Â Â Â Â mask &= AR5K_RESET_CTL_PCU |
> AR5K_RESET_CTL_BASEBAND;
> > + Â Â Â Â Â Â Â }
> > +
> > + Â Â Â Â Â Â Â ret = ath5k_hw_register_timeout(ah, AR5K_RESET_CTL,
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â mask, val, false);
> > + Â Â Â }
> >
> 
> I think it would be much cleaner if we had a different function to
> handle wisoc reset instead
> of putting both on nic_reset. How about having a ath5k_hw_wisoc_reset
> and call that instead ?

I will rework the reset functions and come up with the proposal. The only
problem is that I don't have AR2316 or AR2317 to test all the possible
paths. I have only AR5312/AR2313 and a bunch of miniPCI cards.

I will look whether I can find a cheap one for testing.

Wojtek

> 
> > Â Â Â Â/*
> > Â Â Â Â * Reset configuration register (for hw byte-swap). Note that
> this
> > @@ -334,6 +378,9 @@ int ath5k_hw_on_hold(struct ath5k_hw *ah)
> > Â Â Â Âu32 bus_flags;
> > Â Â Â Âint ret;
> >
> > + Â Â Â if (ath5k_get_bus_type(ah) == ATH_AHB)
> > + Â Â Â Â Â Â Â return 0;
> > +
> > Â Â Â Â/* Make sure device is awake */
> > Â Â Â Âret = ath5k_hw_set_power(ah, AR5K_PM_AWAKE, true, 0);
> > Â Â Â Âif (ret) {
> > @@ -390,22 +437,30 @@ int ath5k_hw_nic_wakeup(struct ath5k_hw *ah,
> int flags, bool initial)
> > Â Â Â Âmode = 0;
> > Â Â Â Âclock = 0;
> >
> > - Â Â Â /* Wakeup the device */
> > - Â Â Â ret = ath5k_hw_set_power(ah, AR5K_PM_AWAKE, true, 0);
> > - Â Â Â if (ret) {
> > - Â Â Â Â Â Â Â ATH5K_ERR(ah->ah_sc, "failed to wakeup the MAC
> Chip\n");
> > - Â Â Â Â Â Â Â return ret;
> > + Â Â Â if (ath5k_get_bus_type(ah) == ATH_AHB && !initial) {
> > + Â Â Â Â Â Â Â /* Wakeup the device */
> > + Â Â Â Â Â Â Â ret = ath5k_hw_set_power(ah, AR5K_PM_AWAKE, true,
> 0);
> > + Â Â Â Â Â Â Â if (ret) {
> > + Â Â Â Â Â Â Â Â Â Â Â ATH5K_ERR(ah->ah_sc, "failed to wakeup the
> MAC Chip\n");
> > + Â Â Â Â Â Â Â Â Â Â Â return ret;
> > + Â Â Â Â Â Â Â }
> > Â Â Â Â}
> >
> 
> You only call ath5k_hw_set_power for AHB devices this way !
> 
> > Â Â Â Â/*
> > Â Â Â Â * Put chipset on warm reset...
> > Â Â Â Â *
> > - Â Â Â Â* Note: putting PCI core on warm reset on PCI-E cards
> > - Â Â Â Â* results card to hang and always return 0xffff... so
> > - Â Â Â Â* we ingore that flag for PCI-E cards. On PCI cards
> > - Â Â Â Â* this flag gets cleared after 64 PCI clocks.
> > Â Â Â Â */
> > - Â Â Â bus_flags = (pdev && pdev->is_pcie) ? 0 :
> AR5K_RESET_CTL_PCI;
> > + Â Â Â if (ath5k_get_bus_type(ah) == ATH_AHB) {
> > + Â Â Â Â Â Â Â /* Reset MAC on WiSoc devices */
> > + Â Â Â Â Â Â Â bus_flags = (initial) ? AR5K_RESET_CTL_MAC : 0;
> > + Â Â Â } else {
> > + Â Â Â Â Â Â Â /* Note: putting PCI core on warm reset on PCI-E
> cards
> > + Â Â Â Â Â Â Â Â* results card to hang and always return 0xffff...
> so
> > + Â Â Â Â Â Â Â Â* we ingore that flag for PCI-E cards. On PCI cards
> > + Â Â Â Â Â Â Â Â* this flag gets cleared after 64 PCI clocks.
> > + Â Â Â Â Â Â Â Â*/
> > + Â Â Â Â Â Â Â bus_flags = (pdev && pdev->is_pcie) ? 0 :
> AR5K_RESET_CTL_PCI;
> > + Â Â Â }
> >
> 
> This is wrong...
> #define AR5K_RESET_CTL_MAC      0x00000004      /* MAC reset
> (PCU+Baseband ?) [5210] */
> this bit was only available on earlier chips. To reset mac on later
> chips (WiSoC too) you need to use
> AR5K_RESET_CTL_PCU  and we already do that below.
> 
> This is something I have to clean up actually, bit 0 is reset mac
> (both PCU + DMA) and bit 1 resets Baseband. Note there is poor
> documentation on that, documentation on AR2317 eg. says mac reset for
> bit 0 and "warm reset to baseband logic" for bit 1 (which is correct)
> but on description for bit 1 says "PCU and DMA but not baseband"
> (that's actually the description of bit 0) and above says "in order to
> reset both baseband and mac and pci write 0x13" (0x10 is pci) that
> doesn't make sense because according to descriptions none of the 2
> first bits resets baseband :P If you go on AR5213, documentation is
> correct, it says that bit 0 is for PCU and DMA and bit 1 is for
> baseband and that also works on all post-5211 chips. That's why I've
> put the comments on reg.h, all [5210] are only available on AR5210,
> only bits/registers marked with [5211+] are available on later macs.
> 
> So what you are really doing here is activate bit 3 that is reserved
> according to docs and should be zeroed ! We already do what's needed
> to reset both mac and baseband later.
> 
> val &= AR5K_RESET_CTL_PCU | AR5K_RESET_CTL_BASEBAND;
> 
> Notice that AR5K_RESET_CTL_PCU | AR5K_RESET_CTL_BASEBAND = 0x13 as
> documentation suggests.
> 

Need to study documentation for this part. Till now it was more of the
trial and error.

Wojtek

> > Â Â Â Âif (ah->ah_version == AR5K_AR5210) {
> > Â Â Â Â Â Â Â Âret = ath5k_hw_nic_reset(ah, AR5K_RESET_CTL_PCU |
> > @@ -536,6 +591,9 @@ static void ath5k_hw_set_sleep_clock(struct
> ath5k_hw *ah, bool enable)
> > Â Â Â Âstruct ath5k_eeprom_info *ee =
> &ah->ah_capabilities.cap_eeprom;
> > Â Â Â Âu32 scal, spending, usec32;
> >
> > + Â Â Â if (ath5k_get_bus_type(ah) == ATH_AHB)
> > + Â Â Â Â Â Â Â enable = false;
> > +
> > Â Â Â Â/* Only set 32KHz settings if we have an external
> > Â Â Â Â * 32KHz crystal present */
> > Â Â Â Âif ((AR5K_EEPROM_HAS32KHZCRYSTAL(ee->ee_misc1) ||
> > @@ -607,6 +665,7 @@ static void ath5k_hw_set_sleep_clock(struct
> ath5k_hw *ah, bool enable)
> >
> > Â Â Â Â Â Â Â Âif ((ah->ah_radio == AR5K_RF5112) ||
> > Â Â Â Â Â Â Â Â(ah->ah_radio == AR5K_RF5413) ||
> > + Â Â Â Â Â Â Â (ah->ah_radio == AR5K_RF2316) ||
> > Â Â Â Â Â Â Â Â(ah->ah_mac_version == (AR5K_SREV_AR2417 >> 4)))
> > Â Â Â Â Â Â Â Â Â Â Â Âspending = 0x14;
> > Â Â Â Â Â Â Â Âelse
> > @@ -614,7 +673,9 @@ static void ath5k_hw_set_sleep_clock(struct
> ath5k_hw *ah, bool enable)
> > Â Â Â Â Â Â Â Âath5k_hw_reg_write(ah, spending, AR5K_PHY_SPENDING);
> >
> > Â Â Â Â Â Â Â Âif ((ah->ah_radio == AR5K_RF5112) ||
> > - Â Â Â Â Â Â Â (ah->ah_radio == AR5K_RF5413))
> > + Â Â Â Â Â Â Â (ah->ah_radio == AR5K_RF5413) ||
> > + Â Â Â Â Â Â Â (ah->ah_radio == AR5K_RF2316) ||
> > + Â Â Â Â Â Â Â (ah->ah_radio == AR5K_RF2317))
> > Â Â Â Â Â Â Â Â Â Â Â Âusec32 = 39;
> > Â Â Â Â Â Â Â Âelse
> > Â Â Â Â Â Â Â Â Â Â Â Âusec32 = 31;
> > @@ -678,7 +739,8 @@ static void
> ath5k_hw_tweak_initval_settings(struct ath5k_hw *ah,
> >
> > Â Â Â Â/* Set fast ADC */
> > Â Â Â Âif ((ah->ah_radio == AR5K_RF5413) ||
> > - Â Â Â (ah->ah_mac_version == (AR5K_SREV_AR2417 >> 4))) {
> > + Â Â Â Â Â Â Â (ah->ah_radio == AR5K_RF2317) ||
> > + Â Â Â Â Â Â Â (ah->ah_mac_version == (AR5K_SREV_AR2417 >> 4))) {
> > Â Â Â Â Â Â Â Âu32 fast_adc = true;
> >
> > Â Â Â Â Â Â Â Âif (channel->center_freq == 2462 ||
> > --
> > 1.7.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> linux-wireless" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at Âhttp://vger.kernel.org/majordomo-info.html
> >
> 
> 
> 
> --
> GPG ID: 0xD21DB2DB
> As you read this post global entropy rises. Have Fun ;-)
> Nick

-- 
Wojciech Dubowik
Senior Software Engineer
Neratec Solutions AG
Rosswiesstr. 29, CH-8608 Bubikon, Switzerland
Tel: +41 55 253 2096 (office)
Fax: +41 55 253 2070
wojciech.dubowik@xxxxxxxxxxx
http://www.neratec.com
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]
  Powered by Linux