Re: [PATCH net-next 0/4] r8152: firmware support
From: Hayes Wang Date: Mon, 25 Aug 2014 06:43:02 + > Except the step 3, 4, 6 and 7, the other steps depend on the > context of the firmware. That is, for different firmware, some > actions would be removed or added, and some settings would be > different. Especially the step 8, it often different for > different firmwares. Should I add some firmware version check > in the source code? This is extremely poor design of the firmware, adding such constantly changing dependencies and constantly changing programming sequences just to get the firmare executing is a terrible idea. You really need to sanitize this in some way, because what you have posted is totally unacceptable to me. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH net-next 0/4] r8152: firmware support
From: David Miller [mailto:da...@davemloft.net]
[...]
> That still doesn't convince me.
>
> The functions I see you removing are just programming a set of
> registers in some way.
That is to clear the break point of the firmware. If a firmware exists,
you should clear it before updating a new one.
> And the firmware that is replacing those functions is just going to be
> causing the same register writes, just even more obfuscated than it is
> now.
>
> You should keep the C functions which document and show clearly what
> is being programmed in each chip.
>
> Don't hide register programming behind firmware files, please.
Excuse me. Some settings are relative the content of the firmware.
How should I deal with that parts. Take 8153 (RTL_VER_03) for
example.
1. wait PLA 0xb800 bit 6 = 1.
2. set patch key = 0x7000.
3. update the PHY firmware.
4. enable the firmware.
5. set USB 0xcfca bit 14 = 0.
6. clear break point (That is r8153_clear_bp()).
7. load the firmware about PLA and USB parts.
8. set the break point of the firmware.
9. set USB 0xcfca bit 14 = 1.
Except the step 3, 4, 6 and 7, the other steps depend on the
context of the firmware. That is, for different firmware, some
actions would be removed or added, and some settings would be
different. Especially the step 8, it often different for
different firmwares. Should I add some firmware version check
in the source code? Such as
if (fw_version == v1) {
...
load firmware
set break point of the firmware
...
} else if (fw_version == v2) {
...--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-next 0/4] r8152: firmware support
From: Hayes Wang Date: Mon, 25 Aug 2014 03:43:04 + > From: David Miller [mailto:da...@davemloft.net] > [...] >> You haven't told us why you need to do this. >> >> These are just programming registers in the chip, and I see no reason >> to not keep these in the driver with real code. >> >> I'm not applying this series, you haven't explained what is happening >> here and the reason for doing so. Ironically, that's exactly what you >> are supposed to provide in this 0/4 header email. > > The nic has the MCU inside which is used to fix the PHY, > MAC, and some behavior of the USB device. Each parts have > different methods of updating the firmware by accessing the > registers. The firmware files are used to deal with the > processes, so I need some functions to parse the firmware > files to update the fimrware code. That still doesn't convince me. The functions I see you removing are just programming a set of registers in some way. And the firmware that is replacing those functions is just going to be causing the same register writes, just even more obfuscated than it is now. You should keep the C functions which document and show clearly what is being programmed in each chip. Don't hide register programming behind firmware files, please. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH net-next 0/4] r8152: firmware support
From: David Miller [mailto:da...@davemloft.net] [...] > You haven't told us why you need to do this. > > These are just programming registers in the chip, and I see no reason > to not keep these in the driver with real code. > > I'm not applying this series, you haven't explained what is happening > here and the reason for doing so. Ironically, that's exactly what you > are supposed to provide in this 0/4 header email. The nic has the MCU inside which is used to fix the PHY, MAC, and some behavior of the USB device. Each parts have different methods of updating the firmware by accessing the registers. The firmware files are used to deal with the processes, so I need some functions to parse the firmware files to update the fimrware code. I would resend these. Sorry. Best Regards, Hayes -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-next 0/4] r8152: firmware support
From: Hayes Wang Date: Wed, 20 Aug 2014 16:58:35 +0800 > Parsing, checking, and writing the firmware. You haven't told us why you need to do this. These are just programming registers in the chip, and I see no reason to not keep these in the driver with real code. I'm not applying this series, you haven't explained what is happening here and the reason for doing so. Ironically, that's exactly what you are supposed to provide in this 0/4 header email. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net-next 0/4] r8152: firmware support
Parsing, checking, and writing the firmware. Hayes Wang (4): r8152: check code with checkpatch.pl r8152: replace strncpy with strlcpy r8152: remove clear_bp function r8152: support firmware files drivers/net/usb/r8152.c | 961 +--- 1 file changed, 903 insertions(+), 58 deletions(-) -- 1.9.3 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
