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

[Petr Štetiar]

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 :-)

> >> + 		/* 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?

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 :-)

-- ynezz

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