Hi Tomi, > -----Original Message----- > From: Tomi Valkeinen [mailto:tomi.valkeinen@xxxxxxxxx] > Sent: Tuesday, September 07, 2010 7:15 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 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. > I shall correct the function names. Yes this is just a RFC patch to introduce the HDMI driver and panel as such, if you have no other comments on these 2 patch set I shall incorporate these comments and send out the complete patch series with all the relevant changes in display.h and some overlay.c and manager.c changes. > Tomi > Thanks and regards, Mythri. -- 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
