RE: [PATCH 2/2] OMAP:DSS: RFC Patch to add HDMI DSS driver support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2010-09-07 at 15:37 +0200, ext K, Mythri P wrote:
> Hi Tomi,
> 
> > -----Original Message-----
> > From: Tomi Valkeinen [mailto:tomi.valkeinen@xxxxxxxxx]
> > Sent: Tuesday, September 07, 2010 6:47 PM
> > To: K, Mythri P
> > Cc: linux-omap@xxxxxxxxxxxxxxx
> > Subject: Re: [PATCH 2/2] OMAP:DSS: RFC Patch to add HDMI DSS driver
> > support
> > 
> > On Thu, 2010-08-26 at 07:17 +0200, ext K, Mythri P wrote:
> > > From: Mythri P K <mythripk@xxxxxx>
> > >
> > > This is an RFC patch to add support for HDMI DSS driver in OMAP.
> > >
> > > Signed-off-by: Mythri P K <mythripk@xxxxxx>
> > > ---
> > >  drivers/video/omap2/dss/hdmi.c |  292
> > ++++++++++++++++++++++++++++++++++++++++
> > >  1 files changed, 292 insertions(+), 0 deletions(-)
> > >  create mode 100644 drivers/video/omap2/dss/hdmi.c
> > >
> > > diff --git a/drivers/video/omap2/dss/hdmi.c
> > b/drivers/video/omap2/dss/hdmi.c
> > > new file mode 100644
> > > index 0000000..3d7acd2
> > > --- /dev/null
> > > +++ b/drivers/video/omap2/dss/hdmi.c
> > > @@ -0,0 +1,292 @@
> > > +/*
> > > + * linux/drivers/video/omap2/dss/hdmi.c
> > > + *
> > > + * Copyright (C) 2009 Texas Instruments
> > > + * Author: Yong Zhi
> > > + *
> > > + * HDMI settings from TI's DSS driver
> > > + *
> > > + * This program is free software; you can redistribute it and/or
> > modify it
> > > + * under the terms of the GNU General Public License version 2 as
> > published by
> > > + * the Free Software Foundation.
> > > + *
> > > + * This program is distributed in the hope that it will be useful,
> > but WITHOUT
> > > + * ANY WARRANTY; without even the implied warranty of
> > MERCHANTABILITY or
> > > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> > License for
> > > + * more details.
> > > + *
> > > + * You should have received a copy of the GNU General Public
> > License along with
> > > + * this program.  If not, see <http://www.gnu.org/licenses/>.
> > > + * History:
> > > + * Mythripk <mythripk@xxxxxx>	-Redesigned on the driver to
> > adhere to DSS2 model.
> > > + *				-GPIO calls for HDMI.
> > > + *
> > > + *
> > > + */
> > > +
> > > +#define DSS_SUBSYS_NAME "HDMI"
> > > +
> > > +#include <linux/kernel.h>
> > > +#include <linux/module.h>
> > > +#include <linux/clk.h>
> > > +#include <linux/err.h>
> > > +#include <linux/io.h>
> > > +#include <linux/interrupt.h>
> > > +#include <linux/mutex.h>
> > > +#include <linux/completion.h>
> > > +#include <linux/delay.h>
> > > +#include <linux/string.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/slab.h>
> > > +#include <plat/display.h>
> > > +#include <plat/cpu.h>
> > > +#include <plat/gpio.h>
> > > +
> > > +#include "dss.h"
> > > +
> > > +static struct {
> > > +	struct mutex lock;
> > > +} hdmi;
> > > +
> > > +#define FLD_GET(val, start, end) (((val) & FLD_MASK(start, end))
> > >> (end))
> > > +#define FLD_MOD(orig, val, start, end) \
> > > +	(((orig) & ~FLD_MASK(start, end)) | FLD_VAL(val, start, end))
> > 
> > These are already defined in dss.h
> > 
> > > +
> > > +
> > > +#define CPF			2
> > > +
> > > +int hdmi_init_display(struct omap_dss_device *dssdev)
> > > +{
> > > +	DSSDBG("init_display\n");
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +void compute_hdmi_pll(int clkin, int phy,
> > > +	int n, struct hdmi_pll_info *pi)
> > 
> > This is a non-static function, and you don't introduce it in any
> > header
> > file.
> > 
> > It is not exported, so it should be used only from inside DSS driver.
> > I
> > see you call it in the next patch from the display driver, which is
> > not
> > ok. Only exported functions should be used from the display drivers
> > (you'll notice the problem if you build DSS as a module...).
> > 
> > If the function is not static, it should have some meaningful prefix.
> > 
> > The same comments apply to some other functions in this file also.
> I have introduced these in the display.h , do you suggest adding prefix like omapdss_hdmi* for all these functions ?

Neither of the two HDMI patches you sent modify display.h...

Yes, if you export functions from DSS they should be prefixed, as they
are global functions.

 Tomi


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux