| Message ID | 20230106160529.1668452-3-netdev@kapio-technology.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | mv88e6xxx: Add MAB offload support | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | fail | Errors and warnings before: 0 this patch: 2 |
| netdev/cc_maintainers | success | CCed 8 of 8 maintainers |
| netdev/build_clang | fail | Errors and warnings before: 0 this patch: 2 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | fail | Errors and warnings before: 0 this patch: 2 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 45 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Fri, Jan 06, 2023 at 05:05:28PM +0100, Hans J. Schultz wrote: > As only the hardware access functions up til and including > mv88e6xxx_g1_atu_mac_read() called under the interrupt handler > need to take the chip lock, we release the chip lock after this call. > The follow up code that handles the violations can run without the > chip lock held. > In further patches, the violation handler function will even be > incompatible with having the chip lock held. This due to an AB/BA > ordering inversion with rtnl_lock(). > > Signed-off-by: Hans J. Schultz <netdev@kapio-technology.com> > --- Needs to compile without new warnings patch by patch. ../drivers/net/dsa/mv88e6xxx/global1_atu.c: In function ‘mv88e6xxx_g1_atu_prob_irq_thread_fn’: ../drivers/net/dsa/mv88e6xxx/global1_atu.c:460:1: warning: label ‘out’ defined but not used [-Wunused-label] 460 | out: | ^~~ ../drivers/net/dsa/mv88e6xxx/global1_atu.c:460:1: warning: unused label 'out'
On 2023-01-06 17:37, Vladimir Oltean wrote: > On Fri, Jan 06, 2023 at 05:05:28PM +0100, Hans J. Schultz wrote: >> As only the hardware access functions up til and including >> mv88e6xxx_g1_atu_mac_read() called under the interrupt handler >> need to take the chip lock, we release the chip lock after this call. >> The follow up code that handles the violations can run without the >> chip lock held. >> In further patches, the violation handler function will even be >> incompatible with having the chip lock held. This due to an AB/BA >> ordering inversion with rtnl_lock(). >> >> Signed-off-by: Hans J. Schultz <netdev@kapio-technology.com> >> --- > > Needs to compile without new warnings patch by patch. > > ../drivers/net/dsa/mv88e6xxx/global1_atu.c: In function > ‘mv88e6xxx_g1_atu_prob_irq_thread_fn’: > ../drivers/net/dsa/mv88e6xxx/global1_atu.c:460:1: warning: label ‘out’ > defined but not used [-Wunused-label] > 460 | out: > | ^~~ > ../drivers/net/dsa/mv88e6xxx/global1_atu.c:460:1: warning: unused label > 'out' Can I fix it and resend the same version?
On Sat, Jan 07, 2023 at 10:44:33AM +0100, netdev@kapio-technology.com wrote:
> Can I fix it and resend the same version?
From my perspective, yes. But not sure what you mean by "the same version".
It will have to be v4.
diff --git a/drivers/net/dsa/mv88e6xxx/global1_atu.c b/drivers/net/dsa/mv88e6xxx/global1_atu.c index 61ae2d61e25c..557428cee457 100644 --- a/drivers/net/dsa/mv88e6xxx/global1_atu.c +++ b/drivers/net/dsa/mv88e6xxx/global1_atu.c @@ -409,23 +409,25 @@ static irqreturn_t mv88e6xxx_g1_atu_prob_irq_thread_fn(int irq, void *dev_id) err = mv88e6xxx_g1_read_atu_violation(chip); if (err) - goto out; + goto out_unlock; err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_ATU_OP, &val); if (err) - goto out; + goto out_unlock; err = mv88e6xxx_g1_atu_fid_read(chip, &fid); if (err) - goto out; + goto out_unlock; err = mv88e6xxx_g1_atu_data_read(chip, &entry); if (err) - goto out; + goto out_unlock; err = mv88e6xxx_g1_atu_mac_read(chip, &entry); if (err) - goto out; + goto out_unlock; + + mv88e6xxx_reg_unlock(chip); spid = entry.state; @@ -449,13 +451,13 @@ static irqreturn_t mv88e6xxx_g1_atu_prob_irq_thread_fn(int irq, void *dev_id) fid); chip->ports[spid].atu_full_violation++; } - mv88e6xxx_reg_unlock(chip); return IRQ_HANDLED; -out: +out_unlock: mv88e6xxx_reg_unlock(chip); +out: dev_err(chip->dev, "ATU problem: error %d while handling interrupt\n", err); return IRQ_HANDLED;
As only the hardware access functions up til and including mv88e6xxx_g1_atu_mac_read() called under the interrupt handler need to take the chip lock, we release the chip lock after this call. The follow up code that handles the violations can run without the chip lock held. In further patches, the violation handler function will even be incompatible with having the chip lock held. This due to an AB/BA ordering inversion with rtnl_lock(). Signed-off-by: Hans J. Schultz <netdev@kapio-technology.com> --- drivers/net/dsa/mv88e6xxx/global1_atu.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)