[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAK9rFnw0s4bVT0TV05EhL+znbJscLmSJKaQkJBtBDKpy+cqE8Q@mail.gmail.com>
Date: Sun, 3 Jul 2022 15:03:41 -0700
From: Brad Larson <brad@...sando.io>
To: Philipp Zabel <p.zabel@...gutronix.de>
Cc: Linux ARM <linux-arm-kernel@...ts.infradead.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
linux-mmc <linux-mmc@...r.kernel.org>,
Adrian Hunter <adrian.hunter@...el.com>,
Al Cooper <alcooperx@...il.com>,
Andy Shevchenko <andy.shevchenko@...il.com>,
Arnd Bergmann <arnd@...db.de>, blarson@....com,
Catalin Marinas <catalin.marinas@....com>,
Gabriel Somlo <gsomlo@...il.com>, gerg@...ux-m68k.org,
Krzysztof Kozlowski <krzk@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Lee Jones <lee.jones@...aro.org>,
Mark Brown <broonie@...nel.org>,
Masahiro Yamada <yamada.masahiro@...ionext.com>,
Pratyush Yadav <p.yadav@...com>,
Randy Dunlap <rdunlap@...radead.org>,
Rob Herring <robh+dt@...nel.org>, samuel@...lland.org,
Serge Semin <fancer.lancer@...il.com>,
suravee.suthikulpanit@....com,
Tom Lendacky <thomas.lendacky@....com>,
Ulf Hansson <ulf.hansson@...aro.org>,
Will Deacon <will@...nel.org>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@...r.kernel.org>
Subject: Re: [PATCH v5 15/15] reset: elbasr: Add AMD Pensando Elba SR Reset Controller
Hi Philipp,
On Tue, Jun 14, 2022 at 7:49 AM Philipp Zabel <p.zabel@...gutronix.de> wrote:
>
> Hi Brad,
>
> On Mo, 2022-06-13 at 12:56 -0700, Brad Larson wrote:
> > From: Brad Larson <blarson@....com>
> >
> > This patch adds the reset controller functionality for the
> > AMD Pensando Elba System Resource Chip.
> >
> > Signed-off-by: Brad Larson <blarson@....com>
> [...]
> > diff --git a/drivers/reset/reset-elbasr.c b/drivers/reset/reset-elbasr.c
> ...
> > +static inline int elbasr_reset_shift(unsigned long id)
> > +{
> > + switch (id) {
> > + case EMMC_HW_RESET:
>
> Are there more reset controls than EMMC_HW_RESET?
> If so, please list them all.
> If not, why is this a function with a switch statement for a single
> reset bit?
>
> > + return 6;
> > + default:
> > + return -EINVAL;
There are others but only emmc hardware reset is currently needed/used. Removed
the switch and just using BIT(6) and removed file amd,pensando-elba-reset.h.
> The error return value is never checked.
> This can't be reached, since ELBASR_NR_RESETS == 1. So id will only
> ever be 0.
>
> > +static int elbasr_reset_probe(struct platform_device *pdev)
> > +{
> > + struct elbasr_data *elbasr = dev_get_drvdata(pdev->dev.parent);
>
> Peeking into the MFD driver's private data structure seems unnecessary.
> Consider using dev_get_regmap() instead.
Prefer to keep it this way as it follows the approach of existing
driver reset-a10sr.c
Regards,
Brad
Powered by blists - more mailing lists