[OpenWrt-Devel] [PATCH] ath79: add support for jjPlus JA76PF2

[Tomasz Maciej Nowak]

W dniu 04.03.2019 o 20:52, Petr Štetiar pisze:
> Tomasz Maciej Nowak <tomek_n at o2.pl> [2019-03-04 19:25:12]:
> 
>>>>  		[ -f "$CONF_TAR" -a "$SAVE_CONFIG" -eq 1 ] && append="-j $CONF_TAR"
>>>>  		dd if="$sysup_file" bs=64k skip=1 2>/dev/null | \
>>>> -			mtd -r $append -Fkernel:$kern_length:0x80060000,rootfs write - kernel:rootfs
>>>> +			mtd -r $append -F$CI_KERNPART:$kern_length:0x80060000,rootfs write - $KERNPART:rootfs
>>>
>>> instead of passing CI_KERNPART as global variable, wouldn't it be better to
>>> pass CI_KERNPART as 2nd argument to this function? Something like this:
>>
>> I just followed an example like in:
>> https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/ipq40xx/base-files/lib/upgrade/platform.sh
>> this is how all other targets pass additional options. Changing this is
>> possible, but I would like to keep it consistent with other implementations.
> 
> I think, that's this isn't exactly good example to follow, but I can see why
> it's done this way, just take a look at package/base-files/files/lib/upgrade/nand.sh
> 
> You don't need to try shoot yourself into the foot :-)
> 

Thanks for pointing that out, I overlooked what was included in the script i pointed to. Version 2 sent.

>>>> + 		/* Didn't find it */
>>>> ++		if (offset + master->erasesize < master->size) {
>>>> ++			/* not at the end of the flash yet, maybe next block :) */
>>>> ++			directory++;
>>>> ++			goto restart;
>>>> ++		}
>>>
>>> I'm wondering if this patch could be upstreamed first, so we don't need to drag it around forever.
>>
>> Yes, that would be preferred, but given my lack of skills and understanding
>> in that regard (no programming skills), I don't see that accepted.
> 
> I would say, that patch is either good enough for upstream, or it has to have
> very good reason to be included and maintained in OpenWrt. So, why does this
> patch belong to OpenWrt if it might not be good enough for upstream?
> 

Would like also to know this, I can assume, since there is option on which block to look for FIS table, available in kernel config, there shouldn't be necessity for this patch. The problem is when one wants to support more boards with same kernel, but those have partition in different place. Anyway, maybe someone from core OpenWrt devs could shed som light on this?

Regarding the part: "I don't see that accepted", I meant if there would be request to change the patch it would be something like this: "Ekh... hmm... how would I do that, don't know any programming.". So I would that patch would stuck in limbo.

> BTW, if you're able to submit patches in this very good quality to OpenWrt,
> I'm pretty sure, that you can do this for kernel as well :-)

Sending patches to OpenWrt is easy :)

> 
> -- ynezz
> 

Regards

--
TMN

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