[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v6 0/3] ppc/e500: Add support for eSDHC
From: |
Daniel Henrique Barboza |
Subject: |
Re: [PATCH v6 0/3] ppc/e500: Add support for eSDHC |
Date: |
Thu, 3 Nov 2022 12:13:22 -0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.3.1 |
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
- [PATCH v6 0/3] ppc/e500: Add support for eSDHC, Philippe Mathieu-Daudé, 2022/11/01
- [PATCH v6 1/3] hw/sd/sdhci: MMIO region is implemented in 32-bit accesses, Philippe Mathieu-Daudé, 2022/11/01
- [PATCH v6 2/3] hw/sd/sdhci: Support big endian SD host controller interfaces, Philippe Mathieu-Daudé, 2022/11/01
- [PATCH v6 3/3] hw/ppc/e500: Add Freescale eSDHC to e500plat, Philippe Mathieu-Daudé, 2022/11/01
- Re: [PATCH v6 0/3] ppc/e500: Add support for eSDHC, Bernhard Beschow, 2022/11/02
- Re: [PATCH v6 0/3] ppc/e500: Add support for eSDHC, Daniel Henrique Barboza, 2022/11/02