On Thu, Sep 11, 2025 at 03:37:07PM +0000, Ferass El Hafidi wrote:
> On Wed Sep 10, 2025 at 8:30 PM UTC, Heinrich Schuchardt wrote:
> > On 9/10/25 22:14, Ferass El Hafidi wrote:
> >> On Wed Sep 10, 2025 at 7:53 PM UTC, Heinrich Schuchardt wrote:
> >>> <...>
> >>>> +static int meson_serial_getc(void)
> >>>> +{
> >>>> + struct meson_uart *const uart = (struct meson_uart
> >>>> *)CONFIG_VAL(DEBUG_UART_BASE);
> >>>> + uint32_t status = readl(&uart->status);
> >>>> +
> >>>> + if (status & AML_UART_RX_EMPTY)
> >>>> + return -EAGAIN;
> >>>> +
> >>>> + if (status & AML_UART_ERR) {
> >>>> + u32 val = readl(&uart->control);
> >>>> +
> >>>> + /* Clear error */
> >>>> + val |= AML_UART_CLR_ERR;
> >>>> + writel(val, &uart->control);
> >>>> + val &= ~AML_UART_CLR_ERR;
> >>>> + writel(val, &uart->control);
> >>>> +
> >>>> + /* Remove spurious byte from fifo */
> >>>> + readl(&uart->rfifo);
> >>>> + return -EIO;
> >>>> + }
> >>>> +
> >>>> + return readl(&uart->rfifo) & 0xff;
> >>>> +}
> >>>> +
> >>>> +static int meson_serial_tstc(void)
> >>>> +{
> >>>> + struct meson_uart *const uart = (struct meson_uart
> >>>> *)CONFIG_VAL(DEBUG_UART_BASE);
> >>>
> >>> CONFIG_SPL_DEBUG_UART_BASE only exists for CONFIG_DEBUG_UART=y and
> >>> CONFIG_SPL=y. Shall all configurations depend on CONFIG_DEBUG_UART=y?
> >>>
> >>> Creating this dependency does not look right to me.
> >>>
> >>
> >> All Amlogic defconfigs have CONFIG_DEBUG_UART enabled, but I can remove
> >> the dependency on CONFIG_DEBUG_UART_BASE if you think it doesn't look
> >> right.
> >
> > We should avoid the possibility of build failures for changed configuration.
> >
> > Will CONFIG_MESON_SERIAL depend on CONFIG_DEBUG_UART
> > or will CONFIG_ARCH_MESON select CONFIG_DEBUG_UART?
>
> Then it might be best to not use DEBUG_UART_BASE. I'll fix that for
> next revision. Thanks!The way we normally solve these kind of things, since they aren't actually configurable (it's part of the chip itself) is just #define them either in the driver header or some <asm/arch/..> header file. -- Tom
signature.asc
Description: PGP signature
