lkml.org 
[lkml]   [2012]   [Apr]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v3 1/3] tegra, serial8250: add ->handle_break() uart_port op
    On 04/09/2012 01:48 PM, Dan Williams wrote:
    > The "KT" serial port has another use case for a "received break" quirk,
    > so before adding another special case to the 8250 core take this
    > opportunity to push such quirks out of the core and into a uart_port op.

    > diff --git a/arch/arm/configs/tegra_defconfig b/arch/arm/configs/tegra_defconfig
    ...
    > +CONFIG_SERIAL_TEGRA=y

    Instead of that,

    > diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
    ...
    > +# FIXME remove this option when Tegra completes conversion to open firmware
    > +config SERIAL_TEGRA
    > + bool "Tegra serial port support"
    > + depends on SERIAL_OF_PLATFORM=y
    > + help
    > + If you have a machine based on NVIDIA Tegra you can enable its
    > + onboard serial ports by enabling this option.
    > +

    Can we just make that default y if ARCH_TEGRA?

    defconfig changes are apparently a little touchy.

    Actually, why even introduce a new config variable; why not replace the
    ifdefs in of_serial.[ch] with CONFIG_ARCH_TEGRA?

    > diff --git a/arch/arm/mach-tegra/board-harmony.c b/arch/arm/mach-tegra/board-harmony.c
    ...
    > static struct platform_device *harmony_devices[] __initdata = {
    > +#if IS_ENABLED(CONFIG_SERIAL_TEGRA)
    > &debug_uart,
    > +#endif

    Yes, it'd be nice to avoid those ifdefs. If you used ARCH_TEGRA instead
    of a new config variable for this, the ifdefs in the board files would
    be guaranteed to be true.

    > diff --git a/arch/arm/mach-tegra/devices.h b/arch/arm/mach-tegra/devices.h
    > index ec45567..6e5f852 100644
    > --- a/arch/arm/mach-tegra/devices.h
    > +++ b/arch/arm/mach-tegra/devices.h
    > @@ -53,5 +53,4 @@ extern struct platform_device tegra_i2s_device1;
    > extern struct platform_device tegra_i2s_device2;
    > extern struct platform_device tegra_das_device;
    > extern struct platform_device tegra_pcm_device;
    > -
    > #endif

    That's left over from a previous patch version.

    > @@ -84,6 +106,9 @@ static int __devinit of_platform_serial_setup(struct platform_device *ofdev,
    > | UPF_FIXED_PORT | UPF_FIXED_TYPE;
    > port->dev = &ofdev->dev;
    >
    > + if (type == PORT_TEGRA)
    > + port->handle_break = tegra_serial_handle_break;

    This is going to mean that everything in 8250.c:uart_config[] will move
    into the exact same data structure in of_serial.c eventually, so I still
    don't see the point of this exercise. But, I guess I won't argue against
    it any more.

    > diff --git a/include/linux/of_serial.h b/include/linux/of_serial.h
    ...
    > + * FIXME remove this file when tegra finishes conversion to open firmware,

    remove this *prototype* not *file*?


    \
     
     \ /
      Last update: 2012-04-10 17:55    [W:0.027 / U:30.912 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site