On Fri Sep 12, 2025 at 11:41 PM UTC, Simon Glass wrote:
> Hi Ferass,
>
> On Thu, 11 Sept 2025 at 09:37, Ferass El Hafidi
> <fundersc...@postmarketos.org> 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!
>
> I've been thinking about doing something about a minimal UART (and
> i2c) for a while.
>
> Before driver model we just called serial_putc() and *the* serial
> driver would handle it. Can we do something similar here?
>
> My suggestion for an API:
>
> CONFIG_SERIAL_ONE to indicate that driver model is not used for serial
>
> Then in serial-uclass.c update serial_putc() to use that config and
> call serial_one_putc(int ch), with that function being defined in the
> only serial driver that is compiled in. Update serial_init() to call
> serial_one_init().
>
> That way we keep things compatible with driver model and avoid
> building on the old serial.c
>
> This is similar to how the debug UART, but the debug UART is special.
Sounds good to me, I'm excited to see a very minimal serial driver API
that doesn't rely on the legacy API, while still being small enough for
SPL.
>
> Regards,
> Simon
