Re: [PULL REQUEST] powerpc generic command line

Hi Daniel,

Le 19/03/2019 à 16:38, Daniel Walker a écrit :

On Tue, Mar 19, 2019 at 12:18:03PM +1100, Michael Ellerman wrote:

quoted

Hi Daniel,

Daniel Walker [off-list ref] writes:

quoted

Here are the generic command line changes for powerpc.

These changes have been in linux-next for two cycles, with few problems reported.
It's also been used at Cisco Systems, Inc. in production products for many many
years with no problems.

Please pull these changes.

Sorry I didn't reply to this earlier, have been busy with merge window
bugs and so on.

As I imagine you noticed, I didn't pull this. There are a few reasons.

Firstly you sent it a bit late, about a day before the 5.0 release, and
at 6am Saturday my time :) In future if you want me to merge something
please send a pull at least the ~Wednesday before the release.

   
Ok .. It was Friday morning my time.

quoted

Secondly I had no idea this code was even in linux-next. I'm not sure if
I was Cc'ed at some point when you added it, if so sorry I missed it,
but I get lots of email. If you're going to add changes to arch/powerpc
in your next tree I'd appreciate some notice, or preferably an explicit
ack.

  
Can I have an ack now ? Since your looking at it. Do you think this has no use,
certainly Cisco has use for it. It's still in linux-next as of now.

quoted

The main reason I didn't merge it is that it's adding a bunch of code
outside of arch/powerpc, into files which I'm not the maintainer for,
and the patches doing so have no acks or reviews from anyone.

With the exception of the Kconfig the header file is brand new, so I'm not sure
who would ack that. From a maintainer perspective I think you could add new
files without issues from other maintainers.

quoted

It's also adding a generic implementation with no indication that any
other arches are willing/able to use the generic implementation, which
begs the question whether it will actually used.

  
It would have been used by powerpc ;) I've gotten feedback in the past from
Ralf Baechle who thought this was useful, however that was years ago when
this was first submitted and the code around this area in mips has changed and
it would require a fair amount of new work to function properly on mips.

Also , no other platforms need to use this. Powerpc could be the only user of
it. This isn't really a question of a new exciting implementation of
something. This is really simple, it's just consolidation across architectures.
The implementation is vanilla, non-exciting stuff.

quoted

I appreciate it's hard to get these sort of cross architecture changes
into mainline, but I don't think this is the way to do it.

I'd suggest you post a patch series to linux-arch with the generic
changes and as many architecture conversions as you can manage, then get
some review/acks for the generic changes and chase arch maintainers for
some acks.

  
I didn't post to linux-arch , but the code has been around for years, submitted
multiple times with more architectures than powerpc. It was scaled down to just
powerpc to simplify it's submission.

It's really a simple set of changes, I don't think it needs as much thought as
other cross architecture changes.

quoted

I realise you have posted the series before, it may require some
persistence. There were also quite a few comments from Christophe, so
replying to those would be a good place to start.

  
I've looked at his comments, but I think he was more worried about conflicts with
his debugging enablement, not something to stop a pull request.

Well, that's what I started with, but at the end my main worry has been 
that you bring a non exciting set of complicated macros and code to 
replace simple code, and you break something out of generic OF code to a 
new brand new generic one, instead of updating the existing generic OF code.

I like the idea behind your series very much, but I don't like too much 
the way it is proposed to be implemented. If you give me one week or 
two, I will come with a lighter proposal that should achieve the same goal.

Christophe

quoted

quoted

The following changes since commit ccda4af0f4b92f7b4c308d3acc262f4a7e3affad:

   Linux 4.20-rc2 (2018-11-11 17:12:31 -0600)

are available in the git repository at:

   https://github.com/daniel-walker/cisco-linux.git for-powerpc

for you to fetch changes up to 5d4514a9c291ecf19b0626695161673d35e5d549:

   powerpc: convert config files to generic cmdline (2018-11-16 07:32:26 -0800)

----------------------------------------------------------------
Daniel Walker (3):
       add generic builtin command line
       powerpc: convert to generic builtin command line
       powerpc: convert config files to generic cmdline

  arch/powerpc/Kconfig                          | 23 +--------
  arch/powerpc/configs/44x/fsp2_defconfig       | 29 ++++++-----
  arch/powerpc/configs/44x/iss476-smp_defconfig | 24 ++++-----
  arch/powerpc/configs/44x/warp_defconfig       | 12 ++---
  arch/powerpc/configs/holly_defconfig          | 12 ++---
  arch/powerpc/configs/mvme5100_defconfig       | 25 +++++-----
  arch/powerpc/configs/skiroot_defconfig        | 48 +++++++++---------
  arch/powerpc/configs/storcenter_defconfig     | 15 +++---

Also if you're updating defconfigs please don't include any unrelated
changes. Trimming the defconfigs can silently drop symbols and break
people's setups so needs to be done carefully.

quoted

It's safer to just sed the defconfig files directly, rather than running
savedefconfig on them.

  
Ok.

Daniel