All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: Talel Shenhar <talel@amazon.com>
Cc: Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Mauro Carvalho Chehab <mchehab+samsung@kernel.org>,
	David Miller <davem@davemloft.net>,
	gregkh <gregkh@linuxfoundation.org>,
	Nicolas Ferre <nicolas.ferre@microchip.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Patrick Venture <venture@google.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Olof Johansson <olof@lixom.net>,
	Maxime Ripard <mripard@kernel.org>,
	Santosh Shilimkar <ssantosh@kernel.org>,
	paul.kocialkowski@bootlin.com, mjourdan@baylibre.com,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>, DTML <devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	David Woodhouse <dwmw@amazon.co.uk>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	hhhawa@amazon.com, ronenk@amazon.com, jonnyc@amazon.com,
	hanochu@amazon.com, barakw@amazon.com
Subject: Re: [PATCH 2/3] soc: amazon: al-pos: Introduce Amazon's Annapurna Labs POS driver
Date: Mon, 9 Sep 2019 11:44:13 +0200	[thread overview]
Message-ID: <CAK8P3a3UF7xPV1U3eW6Jdu754P1bzG208UxD9KUxEm1JjZudww@mail.gmail.com> (raw)
In-Reply-To: <1568020220-7758-3-git-send-email-talel@amazon.com>

On Mon, Sep 9, 2019 at 11:14 AM Talel Shenhar <talel@amazon.com> wrote:
>
> The Amazon's Annapurna Labs SoCs includes Point Of Serialization error
> logging unit that reports an error in case write error (e.g. attempt to
> write to a read only register).
> This patch introduces the support for this unit.
>
> Signed-off-by: Talel Shenhar <talel@amazon.com>

Looks ok overall, juts a few minor comments:

> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Talel Shenhar");
> +MODULE_DESCRIPTION("Amazon's Annapurna Labs POS driver");

These usually go to the end of the file.

> +       log1 = readl_relaxed(pos->mmio_base + AL_POS_ERROR_LOG_1);
> +       if (!FIELD_GET(AL_POS_ERROR_LOG_1_VALID, log1))
> +               return IRQ_NONE;
> +
> +       log0 = readl_relaxed(pos->mmio_base + AL_POS_ERROR_LOG_0);
> +       writel_relaxed(0, pos->mmio_base + AL_POS_ERROR_LOG_1);

Why do you require _relaxed() accessors here? Please add a comment
explaining that, or use the regular readl()/writel().

> +       resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       pos->mmio_base = devm_ioremap_resource(&pdev->dev, resource);

This can be simplified to devm_platform_ioremap_resource().

> +       pos->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);

And this is usually written as platform_get_irq()

       Arnd

WARNING: multiple messages have this Message-ID (diff)
From: Arnd Bergmann <arnd@arndb.de>
To: Talel Shenhar <talel@amazon.com>
Cc: Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Mauro Carvalho Chehab <mchehab+samsung@kernel.org>,
	David Miller <davem@davemloft.net>,
	gregkh <gregkh@linuxfoundation.org>,
	Nicolas Ferre <nicolas.ferre@microchip.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Patrick Venture <venture@google.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Olof Johansson <olof@lixom.net>,
	Maxime Ripard <mripard@kernel.org>,
	Santosh Shilimkar <ssantosh@kernel.org>,
	paul.kocialkowski@bootlin.com, mjourdan@baylibre.com,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>, DTML <devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>David
Subject: Re: [PATCH 2/3] soc: amazon: al-pos: Introduce Amazon's Annapurna Labs POS driver
Date: Mon, 9 Sep 2019 11:44:13 +0200	[thread overview]
Message-ID: <CAK8P3a3UF7xPV1U3eW6Jdu754P1bzG208UxD9KUxEm1JjZudww@mail.gmail.com> (raw)
In-Reply-To: <1568020220-7758-3-git-send-email-talel@amazon.com>

On Mon, Sep 9, 2019 at 11:14 AM Talel Shenhar <talel@amazon.com> wrote:
>
> The Amazon's Annapurna Labs SoCs includes Point Of Serialization error
> logging unit that reports an error in case write error (e.g. attempt to
> write to a read only register).
> This patch introduces the support for this unit.
>
> Signed-off-by: Talel Shenhar <talel@amazon.com>

Looks ok overall, juts a few minor comments:

> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Talel Shenhar");
> +MODULE_DESCRIPTION("Amazon's Annapurna Labs POS driver");

These usually go to the end of the file.

> +       log1 = readl_relaxed(pos->mmio_base + AL_POS_ERROR_LOG_1);
> +       if (!FIELD_GET(AL_POS_ERROR_LOG_1_VALID, log1))
> +               return IRQ_NONE;
> +
> +       log0 = readl_relaxed(pos->mmio_base + AL_POS_ERROR_LOG_0);
> +       writel_relaxed(0, pos->mmio_base + AL_POS_ERROR_LOG_1);

Why do you require _relaxed() accessors here? Please add a comment
explaining that, or use the regular readl()/writel().

> +       resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       pos->mmio_base = devm_ioremap_resource(&pdev->dev, resource);

This can be simplified to devm_platform_ioremap_resource().

> +       pos->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);

And this is usually written as platform_get_irq()

       Arnd

WARNING: multiple messages have this Message-ID (diff)
From: Arnd Bergmann <arnd@arndb.de>
To: Talel Shenhar <talel@amazon.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	mjourdan@baylibre.com, Catalin Marinas <catalin.marinas@arm.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	jonnyc@amazon.com,
	Mauro Carvalho Chehab <mchehab+samsung@kernel.org>,
	ronenk@amazon.com, Will Deacon <will@kernel.org>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	DTML <devicetree@vger.kernel.org>,
	Maxime Ripard <mripard@kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Santosh Shilimkar <ssantosh@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	hanochu@amazon.com,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	barakw@amazon.com, hhhawa@amazon.com,
	gregkh <gregkh@linuxfoundation.org>,
	paul.kocialkowski@bootlin.com,
	Patrick Venture <venture@google.com>,
	Olof Johansson <olof@lixom.net>,
	David Miller <davem@davemloft.net>,
	David Woodhouse <dwmw@amazon.co.uk>
Subject: Re: [PATCH 2/3] soc: amazon: al-pos: Introduce Amazon's Annapurna Labs POS driver
Date: Mon, 9 Sep 2019 11:44:13 +0200	[thread overview]
Message-ID: <CAK8P3a3UF7xPV1U3eW6Jdu754P1bzG208UxD9KUxEm1JjZudww@mail.gmail.com> (raw)
In-Reply-To: <1568020220-7758-3-git-send-email-talel@amazon.com>

On Mon, Sep 9, 2019 at 11:14 AM Talel Shenhar <talel@amazon.com> wrote:
>
> The Amazon's Annapurna Labs SoCs includes Point Of Serialization error
> logging unit that reports an error in case write error (e.g. attempt to
> write to a read only register).
> This patch introduces the support for this unit.
>
> Signed-off-by: Talel Shenhar <talel@amazon.com>

Looks ok overall, juts a few minor comments:

> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Talel Shenhar");
> +MODULE_DESCRIPTION("Amazon's Annapurna Labs POS driver");

These usually go to the end of the file.

> +       log1 = readl_relaxed(pos->mmio_base + AL_POS_ERROR_LOG_1);
> +       if (!FIELD_GET(AL_POS_ERROR_LOG_1_VALID, log1))
> +               return IRQ_NONE;
> +
> +       log0 = readl_relaxed(pos->mmio_base + AL_POS_ERROR_LOG_0);
> +       writel_relaxed(0, pos->mmio_base + AL_POS_ERROR_LOG_1);

Why do you require _relaxed() accessors here? Please add a comment
explaining that, or use the regular readl()/writel().

> +       resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       pos->mmio_base = devm_ioremap_resource(&pdev->dev, resource);

This can be simplified to devm_platform_ioremap_resource().

> +       pos->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);

And this is usually written as platform_get_irq()

       Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2019-09-09  9:44 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-09  9:10 [PATCH 0/3] Amazon's Annapurna Labs POS Driver Talel Shenhar
2019-09-09  9:10 ` Talel Shenhar
2019-09-09  9:10 ` Talel Shenhar
2019-09-09  9:10 ` [PATCH 1/3] dt-bindings: soc: al-pos: Amazon's Annapurna Labs POS Talel Shenhar
2019-09-09  9:10   ` Talel Shenhar
2019-09-09  9:10   ` Talel Shenhar
2019-09-09  9:10 ` [PATCH 2/3] soc: amazon: al-pos: Introduce Amazon's Annapurna Labs POS driver Talel Shenhar
2019-09-09  9:10   ` Talel Shenhar
2019-09-09  9:10   ` Talel Shenhar
2019-09-09  9:44   ` Arnd Bergmann [this message]
2019-09-09  9:44     ` Arnd Bergmann
2019-09-09  9:44     ` Arnd Bergmann
2019-09-09 11:12     ` Shenhar, Talel
2019-09-09 11:12       ` Shenhar, Talel
2019-09-09 11:12       ` Shenhar, Talel
2019-09-09 13:41       ` Arnd Bergmann
2019-09-09 13:41         ` Arnd Bergmann
2019-09-09 13:41         ` Arnd Bergmann
2019-09-09 14:11         ` Shenhar, Talel
2019-09-09 14:11           ` Shenhar, Talel
2019-09-09 14:11           ` Shenhar, Talel
2019-09-09 15:16           ` Arnd Bergmann
2019-09-09 15:16             ` Arnd Bergmann
2019-09-09 15:16             ` Arnd Bergmann
2019-09-10  6:21             ` Shenhar, Talel
2019-09-10  6:21               ` Shenhar, Talel
2019-09-10  6:21               ` Shenhar, Talel
2019-09-09 11:51   ` kbuild test robot
2019-09-09 11:51     ` kbuild test robot
2019-09-09 11:51     ` kbuild test robot
2019-09-09  9:10 ` [PATCH 3/3] arm64: alpine: select AL_POS Talel Shenhar
2019-09-09  9:10   ` Talel Shenhar
2019-09-09  9:10   ` Talel Shenhar
2019-09-09  9:40   ` Arnd Bergmann
2019-09-09  9:40     ` Arnd Bergmann
2019-09-09  9:40     ` Arnd Bergmann
2019-09-09 10:16     ` Shenhar, Talel
2019-09-09 10:16       ` Shenhar, Talel
2019-09-09 10:16       ` Shenhar, Talel
2019-09-09 13:45       ` Arnd Bergmann
2019-09-09 13:45         ` Arnd Bergmann
2019-09-09 13:45         ` Arnd Bergmann
2019-09-09 13:58         ` Shenhar, Talel
2019-09-09 13:58           ` Shenhar, Talel
2019-09-09 13:58           ` Shenhar, Talel
2019-09-09 15:08           ` Arnd Bergmann
2019-09-09 15:08             ` Arnd Bergmann
2019-09-09 15:08             ` Arnd Bergmann
2019-09-10  6:17             ` Shenhar, Talel
2019-09-10  6:17               ` Shenhar, Talel
2019-09-10  6:17               ` Shenhar, Talel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAK8P3a3UF7xPV1U3eW6Jdu754P1bzG208UxD9KUxEm1JjZudww@mail.gmail.com \
    --to=arnd@arndb.de \
    --cc=barakw@amazon.com \
    --cc=benh@kernel.crashing.org \
    --cc=catalin.marinas@arm.com \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=dwmw@amazon.co.uk \
    --cc=gregkh@linuxfoundation.org \
    --cc=hanochu@amazon.com \
    --cc=hhhawa@amazon.com \
    --cc=jonnyc@amazon.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mchehab+samsung@kernel.org \
    --cc=mjourdan@baylibre.com \
    --cc=mripard@kernel.org \
    --cc=nicolas.ferre@microchip.com \
    --cc=olof@lixom.net \
    --cc=paul.kocialkowski@bootlin.com \
    --cc=robh+dt@kernel.org \
    --cc=ronenk@amazon.com \
    --cc=ssantosh@kernel.org \
    --cc=talel@amazon.com \
    --cc=tglx@linutronix.de \
    --cc=venture@google.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.