lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAHp75VexSp1kijGHoOijSHB44fHTWqj6LZPzhSBhN2ms2huh-g@mail.gmail.com>
Date:   Tue, 8 Feb 2022 12:31:49 +0200
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Adrien Thierry <athierry@...hat.com>
Cc:     "open list:SERIAL DRIVERS" <linux-serial@...r.kernel.org>,
        linux-rpi-kernel <linux-rpi-kernel@...ts.infradead.org>,
        linux-arm Mailing List <linux-arm-kernel@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Jeremy Linton <jeremy.linton@....com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jirislaby@...nel.org>,
        Nicolas Saenz Julienne <nsaenz@...nel.org>,
        Florian Fainelli <f.fainelli@...il.com>,
        Ray Jui <rjui@...adcom.com>,
        Scott Branden <sbranden@...adcom.com>,
        bcm-kernel-feedback-list <bcm-kernel-feedback-list@...adcom.com>
Subject: Re: [PATCH v3] serial: 8250_bcm2835aux: Add ACPI support

On Tue, Feb 8, 2022 at 11:13 AM Adrien Thierry <athierry@...hat.com> wrote:
>
> Add ACPI support to 8250_bcm2835aux driver. This makes it possible to
> use the miniuart on the Raspberry Pi with the tianocore/edk2 UEFI

TianoCore EDK II

> firmware.

...

>         /* get the clock - this also enables the HW */
> -       data->clk = devm_clk_get(&pdev->dev, NULL);

> -       if (IS_ERR(data->clk))
> -               return dev_err_probe(&pdev->dev, PTR_ERR(data->clk), "could not get clk\n");

You shouldn't remove the error handling. Even if optional there may be
other types of errors that need to be reported.

> +       data->clk = devm_clk_get_optional(&pdev->dev, NULL);

...


> +       bcm_data = device_get_match_data(&pdev->dev);

Move this closer to the condition where it's used the first time.

> +       /* Some UEFI implementations (e.g. tianocore/edk2 for the Raspberry Pi)
.
If there are some not doing that, can we end up in the situation when
for the same ID we have different offset?
Also, Why not go and fix that implementation?
Can you provide a DSDT excerpt to show how it looks there?

TianoCore EDK II

/*
 * Multi-line comments should look
 * like this.
 */

> +        * describe the miniuart with a base address that encompasses the auxiliary
> +        * registers shared between the miniuart and spi.

SPI

> +        *
> +        * This is due to historical reasons, see discussion here :
> +        * https://edk2.groups.io/g/devel/topic/87501357#84349
> +        *
> +        * We need to add the offset between the miniuart and auxiliary
> +        * registers to get the real miniuart base address.
> +        */
> +       if (bcm_data)
> +               offset = bcm_data->offset;

...

> +static const struct acpi_device_id bcm2835aux_serial_acpi_match[] = {
> +       { "BCM2836", (kernel_ulong_t)&bcm2835_acpi_data },
> +       { }
> +};

I believe this ID is allocated by Broadcom. Can we have a confirmation, please?

-- 
With Best Regards,
Andy Shevchenko

Powered by blists - more mailing lists