[OpenWrt-Devel] [openwrt/openwrt] Revert "ar71xx: ag71xx: Add connect message: fixed phy"

[Daniel F. Dickinson]

On 2018-08-02 01:59 AM, Hannu Nyman wrote:
> Daniel F. Dickinson wrote at Wed Aug 1 14:17:41 PDT 2018:
> 
>> Is this reported to do nasty things *without* debugging?
> 
> 
> Daniel,
> 
> I compiled for WNDR3800 and your commits totally broke the
> switch/interface detection/initialisation. That was with the default
> Openwrt kernel options. No adjustments for debugging.
> 
> Like I wrote in my detailed bug message attached to your original PR and
> referenced in jow's commit message:
> 
> https://github.com/openwrt/openwrt/pull/1217#issuecomment-409708087
> 
> 
> * reverting both your commits fix things in WNDR3800
> 
> * reverting only the OOPS prevention leads into OOPS (no surprise), as
> your first commit adds the dev_info message code likely generating the
> OOPS.
> 

I hadn't see the other notes until after seeing the commit message and
was looking for more info.  After reviewing I'm thinking that the issue
is that the changes break with external switches (since it works with
internal switch).  In any even as I concluded on the note on the PR,
they were supposed to cheap and easy 'nice to have' tweak and aren't
worth spending time on.  I apologize for the trouble they caused.

Regards,

Daniel

> 
> Apparently your first commit tries to give so a detailed error message
> that all needed info is not available and that leads to OOPS. Then your
> second commit tries to prevent the OOPS (which you also noticed??), but
> you placed all your OOPS detection code before the actual tasks of the
> function, so that the actual tasks are not run for some devices.
> 
> Additionally, you adopted a new return code ENODEV that may cause
> unexpected things in the calling function. Earlier this function has
> returned 0 or EINVAL, but you added ENODEV as a return code. I have not
> checked there, so just an idea...
> 
> 
> Things might work if the OOPS prevention code was inside the (!ret)
> condition block where the values are actually needed, and it would not
> invent new return codes. Simply check that the needed data is available
> for the detailed dev_info message that you are creating. If all data is
> not available, write a simpler dev_info message, but do not adjust
> return value of the function itself.
> 
> 


_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel