Hi Philipp,
On Tue, Jun 14, 2022 at 7:49 AM Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> wrote:
>
> Hi Brad,
>
> On Mo, 2022-06-13 at 12:56 -0700, Brad Larson wrote:
> > From: Brad Larson <blarson@xxxxxxx>
> >
> > This patch adds the reset controller functionality for the
> > AMD Pensando Elba System Resource Chip.
> >
> > Signed-off-by: Brad Larson <blarson@xxxxxxx>
> [...]
> > 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
