[PATCH v6 0/3] ppc/e500: Add support for eSDHC

Philippe Mathieu-Daudé posted 3 patches 1 year, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20221101222934.52444-1-philmd@linaro.org
Maintainers: "Philippe Mathieu-Daudé" <philmd@linaro.org>, Bin Meng <bin.meng@windriver.com>
docs/system/ppc/ppce500.rst | 13 ++++++++++
hw/ppc/Kconfig              |  2 ++
hw/ppc/e500.c               | 48 ++++++++++++++++++++++++++++++++++++-
hw/ppc/e500.h               |  1 +
hw/ppc/e500plat.c           |  1 +
hw/sd/sdhci-internal.h      |  1 +
hw/sd/sdhci.c               | 36 +++++++++++++++++++++++++---
include/hw/sd/sdhci.h       |  1 +
8 files changed, 99 insertions(+), 4 deletions(-)
[PATCH v6 0/3] ppc/e500: Add support for eSDHC
Posted by Philippe Mathieu-Daudé 1 year, 5 months ago
This is a respin of Bernhard's v4 with Freescale eSDHC implemented
as an 'UNIMP' region. See v4 cover here:
https://lore.kernel.org/qemu-devel/20221018210146.193159-1-shentey@gmail.com/

Since v5:
- Rebased (ppc-next merged)
- Properly handle big-endian

Since v4:
- Do not rename ESDHC_* definitions to USDHC_*
- Do not modify SDHCIState structure

Supersedes: <20221031115402.91912-1-philmd@linaro.org>

Philippe Mathieu-Daudé (3):
  hw/sd/sdhci: MMIO region is implemented in 32-bit accesses
  hw/sd/sdhci: Support big endian SD host controller interfaces
  hw/ppc/e500: Add Freescale eSDHC to e500plat

 docs/system/ppc/ppce500.rst | 13 ++++++++++
 hw/ppc/Kconfig              |  2 ++
 hw/ppc/e500.c               | 48 ++++++++++++++++++++++++++++++++++++-
 hw/ppc/e500.h               |  1 +
 hw/ppc/e500plat.c           |  1 +
 hw/sd/sdhci-internal.h      |  1 +
 hw/sd/sdhci.c               | 36 +++++++++++++++++++++++++---
 include/hw/sd/sdhci.h       |  1 +
 8 files changed, 99 insertions(+), 4 deletions(-)

-- 
2.38.1


Re: [PATCH v6 0/3] ppc/e500: Add support for eSDHC
Posted by Daniel Henrique Barboza 1 year, 5 months ago

On 11/1/22 19:29, Philippe Mathieu-Daudé wrote:
> This is a respin of Bernhard's v4 with Freescale eSDHC implemented
> as an 'UNIMP' region. See v4 cover here:
> https://lore.kernel.org/qemu-devel/20221018210146.193159-1-shentey@gmail.com/
> 
> Since v5:
> - Rebased (ppc-next merged)
> - Properly handle big-endian
> 
> Since v4:
> - Do not rename ESDHC_* definitions to USDHC_*
> - Do not modify SDHCIState structure
> 
> Supersedes: <20221031115402.91912-1-philmd@linaro.org>

Queued in gitlab.com/danielhb/qemu/tree/ppc-8.0 (since we missed the
freeze for 7.2).


BTW, checkpatch complained about this line being too long (83 chars):


3/3 Checking commit bc7b8cc88560 (hw/ppc/e500: Add Freescale eSDHC to e500plat)
WARNING: line over 80 characters
#150: FILE: hw/ppc/e500.c:1024:
+                                    pmc->ccsrbar_base + MPC85XX_ESDHC_REGS_OFFSET,


The code except is this:

     if (pmc->has_esdhc) {
         create_unimplemented_device("esdhc",
                                     pmc->ccsrbar_base + MPC85XX_ESDHC_REGS_OFFSET,
                                     MPC85XX_ESDHC_REGS_SIZE);


To get rid of the warning we would need to make a python-esque identation (line
break after "(" ) or create a new variable to hold the sum. Both seems overkill
so I'll ignore the warning. Phil is welcome to re-send if he thinks it's worth
it.


And I'll follow it up with my usual plea in these cases: can we move the line size
warning to 100 chars? For QEMU 8.0? Pretty please?


Daniel


> 
> Philippe Mathieu-Daudé (3):
>    hw/sd/sdhci: MMIO region is implemented in 32-bit accesses
>    hw/sd/sdhci: Support big endian SD host controller interfaces
>    hw/ppc/e500: Add Freescale eSDHC to e500plat
> 
>   docs/system/ppc/ppce500.rst | 13 ++++++++++
>   hw/ppc/Kconfig              |  2 ++
>   hw/ppc/e500.c               | 48 ++++++++++++++++++++++++++++++++++++-
>   hw/ppc/e500.h               |  1 +
>   hw/ppc/e500plat.c           |  1 +
>   hw/sd/sdhci-internal.h      |  1 +
>   hw/sd/sdhci.c               | 36 +++++++++++++++++++++++++---
>   include/hw/sd/sdhci.h       |  1 +
>   8 files changed, 99 insertions(+), 4 deletions(-)
> 

Re: [PATCH v6 0/3] ppc/e500: Add support for eSDHC
Posted by BALATON Zoltan 1 year, 5 months ago
On Wed, 2 Nov 2022, Daniel Henrique Barboza wrote:
> On 11/1/22 19:29, Philippe Mathieu-Daudé wrote:
>> This is a respin of Bernhard's v4 with Freescale eSDHC implemented
>> as an 'UNIMP' region. See v4 cover here:
>> https://lore.kernel.org/qemu-devel/20221018210146.193159-1-shentey@gmail.com/
>> 
>> Since v5:
>> - Rebased (ppc-next merged)
>> - Properly handle big-endian
>> 
>> Since v4:
>> - Do not rename ESDHC_* definitions to USDHC_*
>> - Do not modify SDHCIState structure
>> 
>> Supersedes: <20221031115402.91912-1-philmd@linaro.org>
>
> Queued in gitlab.com/danielhb/qemu/tree/ppc-8.0 (since we missed the
> freeze for 7.2).

Could you please always use ppc-next to queue patches for the next 
upcoming version and ppc-7.2 for the current version? Unless this makes 
your workflow harder in which case ignore this but the reason I ask is 
because then it's enough for me to only track ppc-next if I need to rebase 
patches on that and don't have to add a new branch at every release 
(unless I have some patches to rebase on it during a freeze but that's 
less likely than rebasing on your queued patches for the next release xo 
using version for the current branch and keep next for the future versions 
makes more sense to me).

> BTW, checkpatch complained about this line being too long (83 chars):
>
>
> 3/3 Checking commit bc7b8cc88560 (hw/ppc/e500: Add Freescale eSDHC to 
> e500plat)
> WARNING: line over 80 characters
> #150: FILE: hw/ppc/e500.c:1024:
> +                                    pmc->ccsrbar_base + 
> MPC85XX_ESDHC_REGS_OFFSET,
>
>
> The code except is this:
>
>    if (pmc->has_esdhc) {
>        create_unimplemented_device("esdhc",
>                                    pmc->ccsrbar_base + 
> MPC85XX_ESDHC_REGS_OFFSET,
>                                    MPC85XX_ESDHC_REGS_SIZE);
>
>
> To get rid of the warning we would need to make a python-esque identation 
> (line
> break after "(" ) or create a new variable to hold the sum. Both seems 
> overkill
> so I'll ignore the warning. Phil is welcome to re-send if he thinks it's 
> worth
> it.

Or you could break indentation and not start at the ( but 3 chars back. I.e.:

create_unimplemented_device("esdhc",
                          pmc->ccsrbar_base + MPC85XX_ESDHC_REGS_OFFSET,
                          MPC85XX_ESDHC_REGS_SIZE);

But I think it can be just ignored in this case.

> And I'll follow it up with my usual plea in these cases: can we move the line 
> size warning to 100 chars? For QEMU 8.0? Pretty please?

I think the consensus was to keep 80 columns if possible, this is good 
becuase you can open more files side by side (although it does not match 
well with the long _ naming convention of glib and qemu)  but we have a 
distinction between checkpatch warning and error in line length. I think 
it will give error at 90 chars but as long as it's just warns that means: 
fix it if you can but in rare cases if it's more readable with a slightly 
longer line then it is still acceptable. I think that's the case here, 
splitting the line would be less readable than a few chars longer line.

Regards,
BALATON Zoltan
Re: [PATCH v6 0/3] ppc/e500: Add support for eSDHC
Posted by Daniel Henrique Barboza 1 year, 5 months ago

On 11/3/22 09:51, BALATON Zoltan wrote:
> On Wed, 2 Nov 2022, Daniel Henrique Barboza wrote:
>> On 11/1/22 19:29, Philippe Mathieu-Daudé wrote:
>>> This is a respin of Bernhard's v4 with Freescale eSDHC implemented
>>> as an 'UNIMP' region. See v4 cover here:
>>> https://lore.kernel.org/qemu-devel/20221018210146.193159-1-shentey@gmail.com/
>>>
>>> Since v5:
>>> - Rebased (ppc-next merged)
>>> - Properly handle big-endian
>>>
>>> Since v4:
>>> - Do not rename ESDHC_* definitions to USDHC_*
>>> - Do not modify SDHCIState structure
>>>
>>> Supersedes: <20221031115402.91912-1-philmd@linaro.org>
>>
>> Queued in gitlab.com/danielhb/qemu/tree/ppc-8.0 (since we missed the
>> freeze for 7.2).
> 
> Could you please always use ppc-next to queue patches for the next upcoming version and ppc-7.2 for the current version? Unless this makes your workflow harder in which case ignore this but the reason I ask is because then it's enough for me to only track ppc-next if I need to rebase patches on that and don't have to add a new branch at every release (unless I have some patches to rebase on it during a freeze but that's less likely than rebasing on your queued patches for the next release xo using version for the current branch and keep next for the future versions makes more sense to me).

Note that doing "ppc-7.2" for the current release and ppc-next for the
next release will not prevent you from adding a new branch at every
release, e.g. for the next release you would need to add a ppc-8.0
branch.

'ppc-next' is used like a standard, a way of telling 'this is the next
pull request that is going upstream'. Can we change it? Sure, but if the
idea is to avoid new branches every new release then I suggest the
following:

- ppc-next: keep it as is
- ppc-next-release/ppc-future: branch that will host any code for the next
release during the code freeze window. Note that this branch will become
'ppc-next' when the new release cycle begins


This would avoid changing everyone's workflow with the current ppc-next
usage, while also standardize a branch for the future release patches
during freeze.



> 
>> BTW, checkpatch complained about this line being too long (83 chars):
>>
>>
>> 3/3 Checking commit bc7b8cc88560 (hw/ppc/e500: Add Freescale eSDHC to e500plat)
>> WARNING: line over 80 characters
>> #150: FILE: hw/ppc/e500.c:1024:
>> +                                    pmc->ccsrbar_base + MPC85XX_ESDHC_REGS_OFFSET,
>>
>>
>> The code except is this:
>>
>>    if (pmc->has_esdhc) {
>>        create_unimplemented_device("esdhc",
>>                                    pmc->ccsrbar_base + MPC85XX_ESDHC_REGS_OFFSET,
>>                                    MPC85XX_ESDHC_REGS_SIZE);
>>
>>
>> To get rid of the warning we would need to make a python-esque identation (line
>> break after "(" ) or create a new variable to hold the sum. Both seems overkill
>> so I'll ignore the warning. Phil is welcome to re-send if he thinks it's worth
>> it.
> 
> Or you could break indentation and not start at the ( but 3 chars back. I.e.:
> 
> create_unimplemented_device("esdhc",
>                           pmc->ccsrbar_base + MPC85XX_ESDHC_REGS_OFFSET,
>                           MPC85XX_ESDHC_REGS_SIZE);
> 
> But I think it can be just ignored in this case.
> 
>> And I'll follow it up with my usual plea in these cases: can we move the line size warning to 100 chars? For QEMU 8.0? Pretty please?
> 
> I think the consensus was to keep 80 columns if possible, this is good becuase you can open more files side by side (although it does not match well with the long _ naming convention of glib and qemu)  but we have a distinction between checkpatch warning and error in line length. I think it will give error at 90 chars but as long as it's just warns that means: fix it if you can but in rare cases if it's more readable with a slightly longer line then it is still acceptable. I think that's the case here, splitting the line would be less readable than a few chars longer line.

Yeah I know that we can usually ignore these warnings. I keep bringing
this up because it's weird to keep bothering with 80 chars per line when
people are using 28" flat screen monitors, multiple screen desktops
and so on.


Thanks,


Daniel

> 
> Regards,
> BALATON Zoltan

Re: [PATCH v6 0/3] ppc/e500: Add support for eSDHC
Posted by BALATON Zoltan 1 year, 5 months ago
On Thu, 3 Nov 2022, Daniel Henrique Barboza wrote:
> On 11/3/22 09:51, BALATON Zoltan wrote:
>> On Wed, 2 Nov 2022, Daniel Henrique Barboza wrote:
>>> On 11/1/22 19:29, Philippe Mathieu-Daudé wrote:
>>>> This is a respin of Bernhard's v4 with Freescale eSDHC implemented
>>>> as an 'UNIMP' region. See v4 cover here:
>>>> https://lore.kernel.org/qemu-devel/20221018210146.193159-1-shentey@gmail.com/
>>>> 
>>>> Since v5:
>>>> - Rebased (ppc-next merged)
>>>> - Properly handle big-endian
>>>> 
>>>> Since v4:
>>>> - Do not rename ESDHC_* definitions to USDHC_*
>>>> - Do not modify SDHCIState structure
>>>> 
>>>> Supersedes: <20221031115402.91912-1-philmd@linaro.org>
>>> 
>>> Queued in gitlab.com/danielhb/qemu/tree/ppc-8.0 (since we missed the
>>> freeze for 7.2).
>> 
>> Could you please always use ppc-next to queue patches for the next upcoming 
>> version and ppc-7.2 for the current version? Unless this makes your 
>> workflow harder in which case ignore this but the reason I ask is because 
>> then it's enough for me to only track ppc-next if I need to rebase patches 
>> on that and don't have to add a new branch at every release (unless I have 
>> some patches to rebase on it during a freeze but that's less likely than 
>> rebasing on your queued patches for the next release xo using version for 
>> the current branch and keep next for the future versions makes more sense 
>> to me).
>
> Note that doing "ppc-7.2" for the current release and ppc-next for the
> next release will not prevent you from adding a new branch at every
> release, e.g. for the next release you would need to add a ppc-8.0
> branch.
>
> 'ppc-next' is used like a standard, a way of telling 'this is the next
> pull request that is going upstream'. Can we change it? Sure, but if the
> idea is to avoid new branches every new release then I suggest the
> following:
>
> - ppc-next: keep it as is
> - ppc-next-release/ppc-future: branch that will host any code for the next
> release during the code freeze window. Note that this branch will become
> 'ppc-next' when the new release cycle begins
>
>
> This would avoid changing everyone's workflow with the current ppc-next
> usage, while also standardize a branch for the future release patches
> during freeze.

As I said above if this means changing your or other's workflow making it 
more inconvenient for you then just ignore my request as it does not worth 
the trouble this might cause for others. So only change it if it's not 
much trouble.

As for using next for future release and versioned branch for current one 
in freeze this might not completely eliminate the need to track it for me 
but makes it much less likely as I only need the freeze branch when I have 
to submit a fix during the freeze AND you also already have other fixes 
queued AND those fixes conflict with my patch. This is very unlikely so in 
most cases I can just base the fix on master during the freeze and not 
care about the freeze branch only in very rare cases. It's much more 
likely that I have outstanding patches that I have to rebase for the next 
release when you already queued patches e.g. during a freeze (or during 
development before pull requests but the latter case already uses 
ppc-next).

Philippe's solution to use something like ppc-freze, -fixes whatever 
without a version number for pathces during a freeze would also work as 
then I only need to track those two branches but this would also break 
your condition of always using ppc-next for the next pull request so again 
if this causes any trouble for others then just leave it as it is.

>>> BTW, checkpatch complained about this line being too long (83 chars):
>>> 
>>> 
>>> 3/3 Checking commit bc7b8cc88560 (hw/ppc/e500: Add Freescale eSDHC to 
>>> e500plat)
>>> WARNING: line over 80 characters
>>> #150: FILE: hw/ppc/e500.c:1024:
>>> +                                    pmc->ccsrbar_base + 
>>> MPC85XX_ESDHC_REGS_OFFSET,
>>> 
>>> 
>>> The code except is this:
>>> 
>>>    if (pmc->has_esdhc) {
>>>        create_unimplemented_device("esdhc",
>>>                                    pmc->ccsrbar_base + 
>>> MPC85XX_ESDHC_REGS_OFFSET,
>>>                                    MPC85XX_ESDHC_REGS_SIZE);
>>> 
>>> 
>>> To get rid of the warning we would need to make a python-esque identation 
>>> (line
>>> break after "(" ) or create a new variable to hold the sum. Both seems 
>>> overkill
>>> so I'll ignore the warning. Phil is welcome to re-send if he thinks it's 
>>> worth
>>> it.
>> 
>> Or you could break indentation and not start at the ( but 3 chars back. 
>> I.e.:
>> 
>> create_unimplemented_device("esdhc",
>>                           pmc->ccsrbar_base + MPC85XX_ESDHC_REGS_OFFSET,
>>                           MPC85XX_ESDHC_REGS_SIZE);
>> 
>> But I think it can be just ignored in this case.
>> 
>>> And I'll follow it up with my usual plea in these cases: can we move the 
>>> line size warning to 100 chars? For QEMU 8.0? Pretty please?
>> 
>> I think the consensus was to keep 80 columns if possible, this is good 
>> becuase you can open more files side by side (although it does not match 
>> well with the long _ naming convention of glib and qemu)  but we have a 
>> distinction between checkpatch warning and error in line length. I think it 
>> will give error at 90 chars but as long as it's just warns that means: fix 
>> it if you can but in rare cases if it's more readable with a slightly 
>> longer line then it is still acceptable. I think that's the case here, 
>> splitting the line would be less readable than a few chars longer line.
>
> Yeah I know that we can usually ignore these warnings. I keep bringing
> this up because it's weird to keep bothering with 80 chars per line when
> people are using 28" flat screen monitors, multiple screen desktops
> and so on.

Not everyone does. I mostly use a single screen which is not 28" and still 
want to open more than one window without switching desktops or some may 
use laptops with smaller screens etc. 80 chars may be an old convention 
that could be raised now but probably this would just result in some files 
being formatted for longer lines while most of the older code still having 
80 chars so it just brings inconsistency. Reformatting everything would 
not work either so maybe it's easier to just stick with it now.

Regards,
BALATON Zoltan
Re: [PATCH v6 0/3] ppc/e500: Add support for eSDHC
Posted by Daniel Henrique Barboza 1 year, 5 months ago

On 11/3/22 23:31, BALATON Zoltan wrote:
> On Thu, 3 Nov 2022, Daniel Henrique Barboza wrote:
>> On 11/3/22 09:51, BALATON Zoltan wrote:
>>> On Wed, 2 Nov 2022, Daniel Henrique Barboza wrote:
>>>> On 11/1/22 19:29, Philippe Mathieu-Daudé wrote:
>>>>> This is a respin of Bernhard's v4 with Freescale eSDHC implemented
>>>>> as an 'UNIMP' region. See v4 cover here:
>>>>> https://lore.kernel.org/qemu-devel/20221018210146.193159-1-shentey@gmail.com/
>>>>>
>>>>> Since v5:
>>>>> - Rebased (ppc-next merged)
>>>>> - Properly handle big-endian
>>>>>
>>>>> Since v4:
>>>>> - Do not rename ESDHC_* definitions to USDHC_*
>>>>> - Do not modify SDHCIState structure
>>>>>
>>>>> Supersedes: <20221031115402.91912-1-philmd@linaro.org>
>>>>
>>>> Queued in gitlab.com/danielhb/qemu/tree/ppc-8.0 (since we missed the
>>>> freeze for 7.2).
>>>
>>> Could you please always use ppc-next to queue patches for the next upcoming version and ppc-7.2 for the current version? Unless this makes your workflow harder in which case ignore this but the reason I ask is because then it's enough for me to only track ppc-next if I need to rebase patches on that and don't have to add a new branch at every release (unless I have some patches to rebase on it during a freeze but that's less likely than rebasing on your queued patches for the next release xo using version for the current branch and keep next for the future versions makes more sense to me).
>>
>> Note that doing "ppc-7.2" for the current release and ppc-next for the
>> next release will not prevent you from adding a new branch at every
>> release, e.g. for the next release you would need to add a ppc-8.0
>> branch.
>>
>> 'ppc-next' is used like a standard, a way of telling 'this is the next
>> pull request that is going upstream'. Can we change it? Sure, but if the
>> idea is to avoid new branches every new release then I suggest the
>> following:
>>
>> - ppc-next: keep it as is
>> - ppc-next-release/ppc-future: branch that will host any code for the next
>> release during the code freeze window. Note that this branch will become
>> 'ppc-next' when the new release cycle begins
>>
>>
>> This would avoid changing everyone's workflow with the current ppc-next
>> usage, while also standardize a branch for the future release patches
>> during freeze.
> 
> As I said above if this means changing your or other's workflow making it more inconvenient for you then just ignore my request as it does not worth the trouble this might cause for others. So only change it if it's not much trouble.
> 
> As for using next for future release and versioned branch for current one in freeze this might not completely eliminate the need to track it for me but makes it much less likely as I only need the freeze branch when I have to submit a fix during the freeze AND you also already have other fixes queued AND those fixes conflict with my patch. This is very unlikely so in most cases I can just base the fix on master during the freeze and not care about the freeze branch only in very rare cases. It's much more likely that I have outstanding patches that I have to rebase for the next release when you already queued patches e.g. during a freeze (or during development before pull requests but the latter case already uses ppc-next).
> 
> Philippe's solution to use something like ppc-freze, -fixes whatever without a version number for pathces during a freeze would also work as then I only need to track those two branches but this would also break your condition of always using ppc-next for the next pull request so again if this causes any trouble for others then just leave it as it is.


I think I'll actually make the change I proposed:

- ppc-next will always be the next incoming content for the current release,
like it's has been for some time now.

- ppc-future will consist of ppc-next + patches that didn't make the freeze.
And yeah, every change I make on ppc-next I'll rebase ppc-future accordingly.


I won't be using any versioned branch like ppc-7.2, ppc-8.0 or something.
These two branches can be used regardless of the current release number.

I'll do that later today.


Daniel

> 
>>>> BTW, checkpatch complained about this line being too long (83 chars):
>>>>
>>>>
>>>> 3/3 Checking commit bc7b8cc88560 (hw/ppc/e500: Add Freescale eSDHC to e500plat)
>>>> WARNING: line over 80 characters
>>>> #150: FILE: hw/ppc/e500.c:1024:
>>>> +                                    pmc->ccsrbar_base + MPC85XX_ESDHC_REGS_OFFSET,
>>>>
>>>>
>>>> The code except is this:
>>>>
>>>>    if (pmc->has_esdhc) {
>>>>        create_unimplemented_device("esdhc",
>>>>                                    pmc->ccsrbar_base + MPC85XX_ESDHC_REGS_OFFSET,
>>>>                                    MPC85XX_ESDHC_REGS_SIZE);
>>>>
>>>>
>>>> To get rid of the warning we would need to make a python-esque identation (line
>>>> break after "(" ) or create a new variable to hold the sum. Both seems overkill
>>>> so I'll ignore the warning. Phil is welcome to re-send if he thinks it's worth
>>>> it.
>>>
>>> Or you could break indentation and not start at the ( but 3 chars back. I.e.:
>>>
>>> create_unimplemented_device("esdhc",
>>>                           pmc->ccsrbar_base + MPC85XX_ESDHC_REGS_OFFSET,
>>>                           MPC85XX_ESDHC_REGS_SIZE);
>>>
>>> But I think it can be just ignored in this case.
>>>
>>>> And I'll follow it up with my usual plea in these cases: can we move the line size warning to 100 chars? For QEMU 8.0? Pretty please?
>>>
>>> I think the consensus was to keep 80 columns if possible, this is good becuase you can open more files side by side (although it does not match well with the long _ naming convention of glib and qemu)  but we have a distinction between checkpatch warning and error in line length. I think it will give error at 90 chars but as long as it's just warns that means: fix it if you can but in rare cases if it's more readable with a slightly longer line then it is still acceptable. I think that's the case here, splitting the line would be less readable than a few chars longer line.
>>
>> Yeah I know that we can usually ignore these warnings. I keep bringing
>> this up because it's weird to keep bothering with 80 chars per line when
>> people are using 28" flat screen monitors, multiple screen desktops
>> and so on.
> 
> Not everyone does. I mostly use a single screen which is not 28" and still want to open more than one window without switching desktops or some may use laptops with smaller screens etc. 80 chars may be an old convention that could be raised now but probably this would just result in some files being formatted for longer lines while most of the older code still having 80 chars so it just brings inconsistency. Reformatting everything would not work either so maybe it's easier to just stick with it now.
> 
> Regards,
> BALATON Zoltan

Re: [PATCH v6 0/3] ppc/e500: Add support for eSDHC
Posted by Philippe Mathieu-Daudé 1 year, 5 months ago
On 3/11/22 16:13, Daniel Henrique Barboza wrote:
> On 11/3/22 09:51, BALATON Zoltan wrote:
>> On Wed, 2 Nov 2022, Daniel Henrique Barboza wrote:

>>> Queued in gitlab.com/danielhb/qemu/tree/ppc-8.0 (since we missed the
>>> freeze for 7.2).
>>
>> Could you please always use ppc-next to queue patches for the next 
>> upcoming version and ppc-7.2 for the current version? Unless this 
>> makes your workflow harder in which case ignore this but the reason I 
>> ask is because then it's enough for me to only track ppc-next if I 
>> need to rebase patches on that and don't have to add a new branch at 
>> every release (unless I have some patches to rebase on it during a 
>> freeze but that's less likely than rebasing on your queued patches for 
>> the next release xo using version for the current branch and keep next 
>> for the future versions makes more sense to me).
> 
> Note that doing "ppc-7.2" for the current release and ppc-next for the
> next release will not prevent you from adding a new branch at every
> release, e.g. for the next release you would need to add a ppc-8.0
> branch.
> 
> 'ppc-next' is used like a standard, a way of telling 'this is the next
> pull request that is going upstream'. Can we change it? Sure, but if the
> idea is to avoid new branches every new release then I suggest the
> following:
> 
> - ppc-next: keep it as is

FWIW I use mips-next the same way,

> - ppc-next-release/ppc-future: branch that will host any code for the next
> release during the code freeze window. Note that this branch will become
> 'ppc-next' when the new release cycle begins

and use mips-fixes during freeze. My 2 cents, not sure it helps Zoltan.

> This would avoid changing everyone's workflow with the current ppc-next
> usage, while also standardize a branch for the future release patches
> during freeze.
Re: [PATCH v6 0/3] ppc/e500: Add support for eSDHC
Posted by Bernhard Beschow 1 year, 5 months ago
On Tue, Nov 1, 2022 at 11:29 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> This is a respin of Bernhard's v4 with Freescale eSDHC implemented
> as an 'UNIMP' region. See v4 cover here:
>
> https://lore.kernel.org/qemu-devel/20221018210146.193159-1-shentey@gmail.com/
>
> Since v5:
> - Rebased (ppc-next merged)
> - Properly handle big-endian
>

Tested-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Bernhard Beschow <shentey@gmail.com>


> Since v4:
> - Do not rename ESDHC_* definitions to USDHC_*
> - Do not modify SDHCIState structure
>
> Supersedes: <20221031115402.91912-1-philmd@linaro.org>
>
> Philippe Mathieu-Daudé (3):
>   hw/sd/sdhci: MMIO region is implemented in 32-bit accesses
>   hw/sd/sdhci: Support big endian SD host controller interfaces
>   hw/ppc/e500: Add Freescale eSDHC to e500plat
>
>  docs/system/ppc/ppce500.rst | 13 ++++++++++
>  hw/ppc/Kconfig              |  2 ++
>  hw/ppc/e500.c               | 48 ++++++++++++++++++++++++++++++++++++-
>  hw/ppc/e500.h               |  1 +
>  hw/ppc/e500plat.c           |  1 +
>  hw/sd/sdhci-internal.h      |  1 +
>  hw/sd/sdhci.c               | 36 +++++++++++++++++++++++++---
>  include/hw/sd/sdhci.h       |  1 +
>  8 files changed, 99 insertions(+), 4 deletions(-)
>
> --
> 2.38.1
>
>