* Re: [nicholas.johnson-opensource@outlook.com.au: [PATCH v6 4/4] PCI: Add pci=hpmemprefsize parameter to set MMIO_PREF size independently]
[not found] <SL2P216MB018784C16CC1903DF2CEDCB880E50@SL2P216MB0187.KORP216.PROD.OUTLOOK.COM>
@ 2019-06-19 16:45 ` Logan Gunthorpe
2019-06-20 0:56 ` Nicholas Johnson
0 siblings, 1 reply; 8+ messages in thread
From: Logan Gunthorpe @ 2019-06-19 16:45 UTC (permalink / raw)
To: Nicholas Johnson, benh; +Cc: linux-pci, Bjorn Helgaas
On 2019-06-19 8:01 a.m., Nicholas Johnson wrote:
> Hi Ben and Logan,
>
> It looks like my git send-email has been not working correctly since I
> started trying to get these patches accepted. I may have remedied this
> now, but I have seen that Logan tried to find these patches and failed.
> So as a courtesy until I post PATCH v7 (hopefully correctly, this time),
> I am forwarding you the patches. I hope you like them. I would love to
> know of any concerns or questions you may have, and / or what happens if
> you test them. Thanks and all the best!
>
> ----- Forwarded message from Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au> -----
>
> Date: Thu, 23 May 2019 06:29:28 +0800
> From: Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>
> To: linux-kernel@vger.kernel.org
> Cc: linux-pci@vger.kernel.org, bhelgaas@google.com, mika.westerberg@linux.intel.com, corbet@lwn.net, Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>
> Subject: [PATCH v6 4/4] PCI: Add pci=hpmemprefsize parameter to set MMIO_PREF size independently
> X-Mailer: git-send-email 2.19.1
>
> Add kernel parameter pci=hpmemprefsize=nn[KMG] to control MMIO_PREF size
> for PCI hotplug bridges.
Makes sense.
> Change behaviour of pci=hpmemsize=nn[KMG] to not set MMIO_PREF size if
> hpmempref has been specified, rather than controlling both MMIO and
> MMIO_PREF sizes unconditionally.
I don't think I like that fact that hpmemsize behaves differently if
hpmempref size is specfied before it. I'd probably suggest having three
parameters: hpmemsize which sets both as it always has, a pref one and a
regular one which each set one of parameters.
Logan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [nicholas.johnson-opensource@outlook.com.au: [PATCH v6 4/4] PCI: Add pci=hpmemprefsize parameter to set MMIO_PREF size independently]
2019-06-19 16:45 ` [nicholas.johnson-opensource@outlook.com.au: [PATCH v6 4/4] PCI: Add pci=hpmemprefsize parameter to set MMIO_PREF size independently] Logan Gunthorpe
@ 2019-06-20 0:56 ` Nicholas Johnson
2019-06-20 1:35 ` Logan Gunthorpe
0 siblings, 1 reply; 8+ messages in thread
From: Nicholas Johnson @ 2019-06-20 0:56 UTC (permalink / raw)
To: Logan Gunthorpe; +Cc: benh, linux-pci, Bjorn Helgaas
On Wed, Jun 19, 2019 at 10:45:38AM -0600, Logan Gunthorpe wrote:
>
>
> On 2019-06-19 8:01 a.m., Nicholas Johnson wrote:
> > Hi Ben and Logan,
> >
> > It looks like my git send-email has been not working correctly since I
> > started trying to get these patches accepted. I may have remedied this
> > now, but I have seen that Logan tried to find these patches and failed.
> > So as a courtesy until I post PATCH v7 (hopefully correctly, this time),
> > I am forwarding you the patches. I hope you like them. I would love to
> > know of any concerns or questions you may have, and / or what happens if
> > you test them. Thanks and all the best!
> >
> > ----- Forwarded message from Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au> -----
> >
> > Date: Thu, 23 May 2019 06:29:28 +0800
> > From: Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>
> > To: linux-kernel@vger.kernel.org
> > Cc: linux-pci@vger.kernel.org, bhelgaas@google.com, mika.westerberg@linux.intel.com, corbet@lwn.net, Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>
> > Subject: [PATCH v6 4/4] PCI: Add pci=hpmemprefsize parameter to set MMIO_PREF size independently
> > X-Mailer: git-send-email 2.19.1
> >
> > Add kernel parameter pci=hpmemprefsize=nn[KMG] to control MMIO_PREF size
> > for PCI hotplug bridges.
>
> Makes sense.
>
> > Change behaviour of pci=hpmemsize=nn[KMG] to not set MMIO_PREF size if
> > hpmempref has been specified, rather than controlling both MMIO and
> > MMIO_PREF sizes unconditionally.
>
> I don't think I like that fact that hpmemsize behaves differently if
> hpmempref size is specfied before it. I'd probably suggest having three
> parameters: hpmemsize which sets both as it always has, a pref one and a
> regular one which each set one of parameters.
It does not matter if hpmempref is specified before or after hpmemsize.
I made sure of that.
Originally, I proposed to depreciate hpiosize, hpmemsize, and introduce:
hp_io_size, hp_mmio_size, hp_mmio_pref_size, each controlling its own
window exclusively.
The patch had the old parameters work with a warning, and if the new
ones were specified, they would override the old ones. Then, after a few
kernel releases, the old ones could be removed.
Bjorn insisted that there be nil changes which break the existing
parameters, and the solution he requested was to leave hpmemsize to work
exactly the same (controlling both MMIO and MMIO_PREF), unless
hpmemprefsize is given, which will take control of MMIO_PREF from
hpmemsize.
From a maintainer's perspective, I see where he is coming from,
particularly if the changes were to backfire and cause disruptions in
existing setups in the event of a kernel upgrade. What we are left with
is not optimal, but it will work.
(Assuming this is accepted): In the future, if somebody were to come up
with a convincing argument to alter this, the patch to make each
parameter solely control its own window would be miniscule and easy to
sign off.
Cheers
>
> Logan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [nicholas.johnson-opensource@outlook.com.au: [PATCH v6 4/4] PCI: Add pci=hpmemprefsize parameter to set MMIO_PREF size independently]
2019-06-20 0:56 ` Nicholas Johnson
@ 2019-06-20 1:35 ` Logan Gunthorpe
2019-06-20 13:47 ` Bjorn Helgaas
0 siblings, 1 reply; 8+ messages in thread
From: Logan Gunthorpe @ 2019-06-20 1:35 UTC (permalink / raw)
To: Nicholas Johnson; +Cc: benh, linux-pci, Bjorn Helgaas
On 2019-06-19 6:56 p.m., Nicholas Johnson wrote:
> On Wed, Jun 19, 2019 at 10:45:38AM -0600, Logan Gunthorpe wrote:
>>
>>
>> On 2019-06-19 8:01 a.m., Nicholas Johnson wrote:
>>> Hi Ben and Logan,
>>>
>>> It looks like my git send-email has been not working correctly since I
>>> started trying to get these patches accepted. I may have remedied this
>>> now, but I have seen that Logan tried to find these patches and failed.
>>> So as a courtesy until I post PATCH v7 (hopefully correctly, this time),
>>> I am forwarding you the patches. I hope you like them. I would love to
>>> know of any concerns or questions you may have, and / or what happens if
>>> you test them. Thanks and all the best!
>>>
>>> ----- Forwarded message from Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au> -----
>>>
>>> Date: Thu, 23 May 2019 06:29:28 +0800
>>> From: Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>
>>> To: linux-kernel@vger.kernel.org
>>> Cc: linux-pci@vger.kernel.org, bhelgaas@google.com, mika.westerberg@linux.intel.com, corbet@lwn.net, Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>
>>> Subject: [PATCH v6 4/4] PCI: Add pci=hpmemprefsize parameter to set MMIO_PREF size independently
>>> X-Mailer: git-send-email 2.19.1
>>>
>>> Add kernel parameter pci=hpmemprefsize=nn[KMG] to control MMIO_PREF size
>>> for PCI hotplug bridges.
>>
>> Makes sense.
>>
>>> Change behaviour of pci=hpmemsize=nn[KMG] to not set MMIO_PREF size if
>>> hpmempref has been specified, rather than controlling both MMIO and
>>> MMIO_PREF sizes unconditionally.
>>
>> I don't think I like that fact that hpmemsize behaves differently if
>> hpmempref size is specfied before it. I'd probably suggest having three
>> parameters: hpmemsize which sets both as it always has, a pref one and a
>> regular one which each set one of parameters.
>
> It does not matter if hpmempref is specified before or after hpmemsize.
> I made sure of that.
> Originally, I proposed to depreciate hpiosize, hpmemsize, and introduce:
> hp_io_size, hp_mmio_size, hp_mmio_pref_size, each controlling its own
> window exclusively.
>
> The patch had the old parameters work with a warning, and if the new
> ones were specified, they would override the old ones. Then, after a few
> kernel releases, the old ones could be removed.
Well I don't like that either. No need to depreciate hpmemsize.
> Bjorn insisted that there be nil changes which break the existing
> parameters, and the solution he requested was to leave hpmemsize to work
> exactly the same (controlling both MMIO and MMIO_PREF), unless
> hpmemprefsize is given, which will take control of MMIO_PREF from
> hpmemsize.
I agree with Bjorn here too but my suggestion is to leave hpmemsize
alone and have it set both values as it has always done. And add two new
parameters to set one or the other. Then there's none of this "sets one
if the other one wasn't set". Also, if I only want to change the
non-preftechable version then your method leaves no way to do so without
setting the preftechable version.
Logan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [nicholas.johnson-opensource@outlook.com.au: [PATCH v6 4/4] PCI: Add pci=hpmemprefsize parameter to set MMIO_PREF size independently]
2019-06-20 1:35 ` Logan Gunthorpe
@ 2019-06-20 13:47 ` Bjorn Helgaas
2019-06-21 2:57 ` Nicholas Johnson
0 siblings, 1 reply; 8+ messages in thread
From: Bjorn Helgaas @ 2019-06-20 13:47 UTC (permalink / raw)
To: Logan Gunthorpe; +Cc: Nicholas Johnson, benh, linux-pci
On Wed, Jun 19, 2019 at 07:35:21PM -0600, Logan Gunthorpe wrote:
> On 2019-06-19 6:56 p.m., Nicholas Johnson wrote:
> > On Wed, Jun 19, 2019 at 10:45:38AM -0600, Logan Gunthorpe wrote:
> >> On 2019-06-19 8:01 a.m., Nicholas Johnson wrote:
> >>> ----- Forwarded message from Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au> -----
> >>>
> >>> Date: Thu, 23 May 2019 06:29:28 +0800
> >>> From: Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>
> >>> To: linux-kernel@vger.kernel.org
> >>> Cc: linux-pci@vger.kernel.org, bhelgaas@google.com, mika.westerberg@linux.intel.com, corbet@lwn.net, Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>
> >>> Subject: [PATCH v6 4/4] PCI: Add pci=hpmemprefsize parameter to set MMIO_PREF size independently
> >>> X-Mailer: git-send-email 2.19.1
> >>>
> >>> Add kernel parameter pci=hpmemprefsize=nn[KMG] to control
> >>> MMIO_PREF size for PCI hotplug bridges.
> >>
> >> Makes sense.
> >>
> >>> Change behaviour of pci=hpmemsize=nn[KMG] to not set MMIO_PREF
> >>> size if hpmempref has been specified, rather than controlling
> >>> both MMIO and MMIO_PREF sizes unconditionally.
> >>
> >> I don't think I like that fact that hpmemsize behaves differently
> >> if hpmempref size is specfied before it. I'd probably suggest
> >> having three parameters: hpmemsize which sets both as it always
> >> has, a pref one and a regular one which each set one of
> >> parameters.
> >
> > It does not matter if hpmempref is specified before or after
> > hpmemsize. I made sure of that.
>
> > Originally, I proposed to depreciate hpiosize, hpmemsize, and
> > introduce: hp_io_size, hp_mmio_size, hp_mmio_pref_size, each
> > controlling its own window exclusively.
> >
> > The patch had the old parameters work with a warning, and if the
> > new ones were specified, they would override the old ones. Then,
> > after a few kernel releases, the old ones could be removed.
>
> Well I don't like that either. No need to depreciate hpmemsize.
>
> > Bjorn insisted that there be nil changes which break the existing
> > parameters, and the solution he requested was to leave hpmemsize
> > to work exactly the same (controlling both MMIO and MMIO_PREF),
> > unless hpmemprefsize is given, which will take control of
> > MMIO_PREF from hpmemsize.
>
> I agree with Bjorn here too but my suggestion is to leave hpmemsize
> alone and have it set both values as it has always done. And add two
> new parameters to set one or the other. Then there's none of this
> "sets one if the other one wasn't set". Also, if I only want to
> change the non-preftechable version then your method leaves no way
> to do so without setting the preftechable version.
Adding two new parameters sounds like a good idea to me.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [nicholas.johnson-opensource@outlook.com.au: [PATCH v6 4/4] PCI: Add pci=hpmemprefsize parameter to set MMIO_PREF size independently]
2019-06-20 13:47 ` Bjorn Helgaas
@ 2019-06-21 2:57 ` Nicholas Johnson
2019-06-21 5:11 ` Logan Gunthorpe
0 siblings, 1 reply; 8+ messages in thread
From: Nicholas Johnson @ 2019-06-21 2:57 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Logan Gunthorpe, benh, linux-pci
On Thu, Jun 20, 2019 at 08:47:12AM -0500, Bjorn Helgaas wrote:
> On Wed, Jun 19, 2019 at 07:35:21PM -0600, Logan Gunthorpe wrote:
> > On 2019-06-19 6:56 p.m., Nicholas Johnson wrote:
> > > On Wed, Jun 19, 2019 at 10:45:38AM -0600, Logan Gunthorpe wrote:
> > >> On 2019-06-19 8:01 a.m., Nicholas Johnson wrote:
> > >>> ----- Forwarded message from Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au> -----
> > >>>
> > >>> Date: Thu, 23 May 2019 06:29:28 +0800
> > >>> From: Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>
> > >>> To: linux-kernel@vger.kernel.org
> > >>> Cc: linux-pci@vger.kernel.org, bhelgaas@google.com, mika.westerberg@linux.intel.com, corbet@lwn.net, Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>
> > >>> Subject: [PATCH v6 4/4] PCI: Add pci=hpmemprefsize parameter to set MMIO_PREF size independently
> > >>> X-Mailer: git-send-email 2.19.1
> > >>>
> > >>> Add kernel parameter pci=hpmemprefsize=nn[KMG] to control
> > >>> MMIO_PREF size for PCI hotplug bridges.
> > >>
> > >> Makes sense.
> > >>
> > >>> Change behaviour of pci=hpmemsize=nn[KMG] to not set MMIO_PREF
> > >>> size if hpmempref has been specified, rather than controlling
> > >>> both MMIO and MMIO_PREF sizes unconditionally.
> > >>
> > >> I don't think I like that fact that hpmemsize behaves differently
> > >> if hpmempref size is specfied before it. I'd probably suggest
> > >> having three parameters: hpmemsize which sets both as it always
> > >> has, a pref one and a regular one which each set one of
> > >> parameters.
> > >
> > > It does not matter if hpmempref is specified before or after
> > > hpmemsize. I made sure of that.
> >
> > > Originally, I proposed to depreciate hpiosize, hpmemsize, and
> > > introduce: hp_io_size, hp_mmio_size, hp_mmio_pref_size, each
> > > controlling its own window exclusively.
> > >
> > > The patch had the old parameters work with a warning, and if the
> > > new ones were specified, they would override the old ones. Then,
> > > after a few kernel releases, the old ones could be removed.
> >
> > Well I don't like that either. No need to depreciate hpmemsize.
> >
> > > Bjorn insisted that there be nil changes which break the existing
> > > parameters, and the solution he requested was to leave hpmemsize
> > > to work exactly the same (controlling both MMIO and MMIO_PREF),
> > > unless hpmemprefsize is given, which will take control of
> > > MMIO_PREF from hpmemsize.
> >
> > I agree with Bjorn here too but my suggestion is to leave hpmemsize
> > alone and have it set both values as it has always done. And add two
> > new parameters to set one or the other. Then there's none of this
> > "sets one if the other one wasn't set". Also, if I only want to
> > change the non-preftechable version then your method leaves no way
> > to do so without setting the preftechable version.
>
> Adding two new parameters sounds like a good idea to me.
Yeah, that is basically what I did originally (except I depreciated the
old ones rather than keeping them).
I did it this way on your direct advice in keeping with minimal lines of
diff, minimal disruption, etc. If I were to do this, the number of lines
of diff will increase and then I will be fielding complaints that it is
too large to sign off.
I am already scrambling to make last minute changes before end of
release to the other patches and I am not even convinced that that stuff
is going to get accepted based on proximity to deadline and how many
change requests are flying around.
So I am going to have to respectfully decline to do this for now. I need
to know earlier in the release cycle if I am going to go back on stuff I
have already been asked to do.
Cheers.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [nicholas.johnson-opensource@outlook.com.au: [PATCH v6 4/4] PCI: Add pci=hpmemprefsize parameter to set MMIO_PREF size independently]
2019-06-21 2:57 ` Nicholas Johnson
@ 2019-06-21 5:11 ` Logan Gunthorpe
2019-06-21 6:01 ` Nicholas Johnson
0 siblings, 1 reply; 8+ messages in thread
From: Logan Gunthorpe @ 2019-06-21 5:11 UTC (permalink / raw)
To: Nicholas Johnson, Bjorn Helgaas; +Cc: benh, linux-pci
On 2019-06-20 8:57 p.m., Nicholas Johnson wrote:
>> Adding two new parameters sounds like a good idea to me.
> Yeah, that is basically what I did originally (except I depreciated the
> old ones rather than keeping them).
>
> I did it this way on your direct advice in keeping with minimal lines of
> diff, minimal disruption, etc. If I were to do this, the number of lines
> of diff will increase and then I will be fielding complaints that it is
> too large to sign off.
>
> I am already scrambling to make last minute changes before end of
> release to the other patches and I am not even convinced that that stuff
> is going to get accepted based on proximity to deadline and how many
> change requests are flying around.
Friendly advice: Linux Kernel development doesn't really work on
deadlines. The patch I linked you to has already been around a couple of
cycles and has missed a couple of merge windows. It's not that big of a
deal. I try to make it better, if I can, and resubmit it once or twice
a cycle. It will get in when other people understand it and it meets the
community's standards. I had one patch set I submitted for more than a
year and a half, or 25 times, and it eventually got picked up. It's not
always the best experience but patience, persistence and openness to
feedback are what works.
New kernel parameters are important to get right because they are user
facing interfaces and we are stuck with them forever -- breaking
existing users is simply not accepted here. Deprecating features is an
extreme action that the Linux community takes pains to avoid. If we cede
to deadlines to get a new parameter in, and it turns out to be
non-ideal, then we are stuck supporting it forever and it's painful for
everyone.
Logan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [nicholas.johnson-opensource@outlook.com.au: [PATCH v6 4/4] PCI: Add pci=hpmemprefsize parameter to set MMIO_PREF size independently]
2019-06-21 5:11 ` Logan Gunthorpe
@ 2019-06-21 6:01 ` Nicholas Johnson
2019-06-21 16:03 ` Logan Gunthorpe
0 siblings, 1 reply; 8+ messages in thread
From: Nicholas Johnson @ 2019-06-21 6:01 UTC (permalink / raw)
To: Logan Gunthorpe; +Cc: Bjorn Helgaas, benh, linux-pci
On Thu, Jun 20, 2019 at 11:11:05PM -0600, Logan Gunthorpe wrote:
>
>
> On 2019-06-20 8:57 p.m., Nicholas Johnson wrote:
> >> Adding two new parameters sounds like a good idea to me.
> > Yeah, that is basically what I did originally (except I depreciated the
> > old ones rather than keeping them).
> >
> > I did it this way on your direct advice in keeping with minimal lines of
> > diff, minimal disruption, etc. If I were to do this, the number of lines
> > of diff will increase and then I will be fielding complaints that it is
> > too large to sign off.
> >
> > I am already scrambling to make last minute changes before end of
> > release to the other patches and I am not even convinced that that stuff
> > is going to get accepted based on proximity to deadline and how many
> > change requests are flying around.
>
> Friendly advice: Linux Kernel development doesn't really work on
> deadlines. The patch I linked you to has already been around a couple of
> cycles and has missed a couple of merge windows. It's not that big of a
> deal. I try to make it better, if I can, and resubmit it once or twice
> a cycle. It will get in when other people understand it and it meets the
> community's standards. I had one patch set I submitted for more than a
> year and a half, or 25 times, and it eventually got picked up. It's not
> always the best experience but patience, persistence and openness to
> feedback are what works.
>
> New kernel parameters are important to get right because they are user
> facing interfaces and we are stuck with them forever -- breaking
> existing users is simply not accepted here. Deprecating features is an
> extreme action that the Linux community takes pains to avoid. If we cede
> to deadlines to get a new parameter in, and it turns out to be
> non-ideal, then we are stuck supporting it forever and it's painful for
> everyone.
>
> Logan
Thanks for advice. I do believe you are correct. I guess it is my
inexperience - I have trouble telling what is normal and what to expect
from the process. But I am not feeling like this is any closer to
submission than on day one - no sense of progress. It is not reassuring
and I do wonder if I will ever manage to get it right.
None of this is technically Thunderbolt-specific, but that is the use
case, and if a single one of these patches does not make it in then the
use case of Thunderbolt without firmware support will remain broken. The
only benefit of a partial acception is patch #1 which fixes a bug report
by Mika Westerberg on its own, so I guess that would be a win. If it has
to wait another cycle then I will live with it, but this is the cause of
my anxiety.
I can look at this patch again, but my concerns are the following:
- If we avoid depreciation, then that implies this patch is ideal - we
are not breaking anything for existing users. We can set MMIO on its own
with pci=hpmemsize and by setting pci=hpmemprefsize to the default size of
2M (so it remains unchanged).
- If we have two sets of parameters, then we have to support having two
sets for eternity, and the loss of minimalism and the potential for
confusion. The description in kernel-parameters.txt will become
confusing with a lot of "if this" and "if that".
- How do we deal with order if both are specified? It will become
unclean.
- If we make two new ones, then what should they be called? I propose
hp_mmio_size and hp_mmio_pref_size from my original submission. Then
should hpiosize be aliased to hp_io_size to keep the consistency?
- I am quite surprised that when 64-bit window support was added to the
kernel, that this was not done then, although I am not good enough at
navigating the mailing list to see what actually happened.
Should I perhaps forward my original submission for this patch again and
you can see if there is anything to salvage from that that?
Cheers
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [nicholas.johnson-opensource@outlook.com.au: [PATCH v6 4/4] PCI: Add pci=hpmemprefsize parameter to set MMIO_PREF size independently]
2019-06-21 6:01 ` Nicholas Johnson
@ 2019-06-21 16:03 ` Logan Gunthorpe
0 siblings, 0 replies; 8+ messages in thread
From: Logan Gunthorpe @ 2019-06-21 16:03 UTC (permalink / raw)
To: Nicholas Johnson; +Cc: Bjorn Helgaas, benh, linux-pci
On 2019-06-21 12:01 a.m., Nicholas Johnson wrote:
> Thanks for advice. I do believe you are correct. I guess it is my
> inexperience - I have trouble telling what is normal and what to expect
> from the process. But I am not feeling like this is any closer to
> submission than on day one - no sense of progress. It is not reassuring
> and I do wonder if I will ever manage to get it right.
>
> None of this is technically Thunderbolt-specific, but that is the use
> case, and if a single one of these patches does not make it in then the
> use case of Thunderbolt without firmware support will remain broken. The
> only benefit of a partial acception is patch #1 which fixes a bug report
> by Mika Westerberg on its own, so I guess that would be a win. If it has
> to wait another cycle then I will live with it, but this is the cause of
> my anxiety.
>
> I can look at this patch again, but my concerns are the following:
>
> - If we avoid depreciation, then that implies this patch is ideal - we
> are not breaking anything for existing users. We can set MMIO on its own
> with pci=hpmemsize and by setting pci=hpmemprefsize to the default size of
> 2M (so it remains unchanged).
I disagree. The semantics are weird and the changes to the documentation
show that. When you have to describe things based on what other
parameters are doing, it's weird.
> - If we have two sets of parameters, then we have to support having two
> sets for eternity, and the loss of minimalism and the potential for
> confusion. The description in kernel-parameters.txt will become
> confusing with a lot of "if this" and "if that".
Yes, but they're simple and clear parameters that are easy to support.
There would be no "ifs" at all. One parameter sets one value, another a
parameter sets another, and the original parameter sets both.
> - How do we deal with order if both are specified? It will become
> unclean.
If a user mixes the "both" with the "single" parameters, the winner goes
to the last that's specified. This is common and predictable. It's not
dissimilar to when a user specifies the same parameter twice.
> - If we make two new ones, then what should they be called? I propose
> hp_mmio_size and hp_mmio_pref_size from my original submission. Then
> should hpiosize be aliased to hp_io_size to keep the consistency?
Good question. I'd probably go something along the lines of
'hpmemsize_pref' and 'hpmemsize_nonpref'. Though, it might be good to
get others opinions on this.
> - I am quite surprised that when 64-bit window support was added to the
> kernel, that this was not done then, although I am not good enough at
> navigating the mailing list to see what actually happened.
Generally, things won't get done until people have a use case for them
and motivation to make the change. I don't think there's a lot of users
of hpmemsize to begin with.
> Should I perhaps forward my original submission for this patch again and
> you can see if there is anything to salvage from that that?
Not if you haven't changed anything.
Logan
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-06-21 16:03 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <SL2P216MB018784C16CC1903DF2CEDCB880E50@SL2P216MB0187.KORP216.PROD.OUTLOOK.COM>
2019-06-19 16:45 ` [nicholas.johnson-opensource@outlook.com.au: [PATCH v6 4/4] PCI: Add pci=hpmemprefsize parameter to set MMIO_PREF size independently] Logan Gunthorpe
2019-06-20 0:56 ` Nicholas Johnson
2019-06-20 1:35 ` Logan Gunthorpe
2019-06-20 13:47 ` Bjorn Helgaas
2019-06-21 2:57 ` Nicholas Johnson
2019-06-21 5:11 ` Logan Gunthorpe
2019-06-21 6:01 ` Nicholas Johnson
2019-06-21 16:03 ` Logan Gunthorpe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).