Message-ID: <CAAfyv36MaVqrmQxpqDu-ciO9GNJK3pJkFZohEvd1BOLOWWEOhw@mail.gmail.com>
Date:	Tue, 10 Dec 2013 23:11:57 +0100
From:	Belisko Marek <marek.belisko@...il.com>
To:	Tony Lindgren <tony@...mide.com>
Cc:	Russell King - ARM Linux <linux@....linux.org.uk>,
	Tomi Valkeinen <tomi.valkeinen@...com>,
	Jean-Christophe PLAGNIOL-VILLARD <plagnioj@...osoft.com>,
	linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
	"linux-omap@...r.kernel.org" <linux-omap@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>, linux-fbdev@...r.kernel.org,
	NeilBrown <neilb@...e.de>,
	"Dr. H. Nikolaus Schaller" <hns@...delico.com>
Subject: Re: [PATCH 1/3] arm: omap2: Export devconf1 bypass and acbias.

Hi Tony,

On Tue, Nov 12, 2013 at 12:31 AM, Tony Lindgren <tony@...mide.com> wrote:
> * Belisko Marek <marek.belisko@...il.com> [131111 14:01]:
>> Hi Tony,
>>
>> On Mon, Nov 11, 2013 at 5:49 PM, Tony Lindgren <tony@...mide.com> wrote:
>> > * Marek Belisko <marek@...delico.com> [131014 14:11]:
>> >> devconf1 reg access is localized only in mach-omap2 and we need to export
>> >> updating of devconf1 from omapdss venc driver (bypass and acbias bits).
>> >> Add simple api call which update only necessary bits.
>> > ...
>> >
>> >> +void update_bypass_acbias(bool bypass, bool acbias)
>> >> +{
>> >> +#ifdef CONFIG_ARCH_OMAP3
>> >> +     int val = omap_ctrl_readl(OMAP343X_CONTROL_DEVCONF1);
>> >> +
>> >> +     if (bypass)
>> >> +             val |= OMAP2_TVOUTBYPASS;
>> >> +     else
>> >> +             val &= ~OMAP2_TVOUTBYPASS;
>> >> +
>> >> +     if (acbias)
>> >> +             val |= OMAP2_TVACEN;
>> >> +     else
>> >> +             val &= ~OMAP2_TVACEN;
>> >> +
>> >> +     omap_ctrl_writel(val, OMAP343X_CONTROL_DEVCONF1);
>> >> +#endif
>> >
>> > If this is truly a pinmux, you could already access this
>> > using pinctrl-single,bits device tree driver.
>> >
>> > But I guess that won't work yet, so it's best to set this
>> > up as a separate driver like we've done for the USB PHY
>> > registers.
>>
>> Can you please point me to that driver directly? I cannot see benefit
>> of new driver as as it will be only dummy driver
>> with export_symbol_gpl of upper function. Can it sit then in dss
>> directory or somewhere else? Thanks.
>
> You can take a look at drivers/usb/phy/phy-omap-control.c.
>
> Then there's a device tree using example for control_devconf0 in
> Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt.
>
> Looks like CONTROL_DEVCONF1 contains a bunch of misc registers,
> and arch/arm/mach-omap2/hsmmc.c seems to be tinkering with it too.
>
> It would be best to set it up as omap-ctrl.c driver under drivers
> somewhere with few functions exported for DSS and MMC drivers.
I create small dummy driver based on phy-omap-control which can be
used but before sending patch (with more updates) I would like to get
some feedback if my direction is correct.
diff --git a/drivers/misc/omap-ctrl.c b/drivers/misc/omap-ctrl.c
new file mode 100644
index 0000000..ca2e576
--- /dev/null
+++ b/drivers/misc/omap-ctrl.c
@@ -0,0 +1,84 @@
+/*
+ * omap-ctrl.c - The part of omap control module.
+ *
+ * Copyright (C) 2013 Goldelico GmbH
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * Author: Marek Belisko <marek@...delico.com>
+ *
+ * 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.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+
+static u32 __iomem *devconf1;
+
+void omap_ctrl_tvoutbypass(int on)
+{
+    int val = readl(devconf1);
+
+    if (on)
+       val |= BIT(18);
+    else
+       val &= ~BIT(18);
+
+    writel(val, devconf1);
+}
+EXPORT_SYMBOL_GPL(omap_ctrl_tvoutbypass);
+
+
+void omap_ctrl_acen(int on)
+{
+    int val = readl(devconf1);
+
+    if (on)
+       val |= BIT(11);
+    else
+       val &= ~BIT(11);
+
+    writel(val, devconf1);
+}
+EXPORT_SYMBOL_GPL(omap_ctrl_acen);
+
+
+static int omap_ctrl_probe(struct platform_device *pdev)
+{
+    struct resource *res;
+
+    res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+       "devconf1");
+    devconf1 = devm_ioremap_resource(
+       &pdev->dev, res);
+    if (IS_ERR(devconf1))
+       return PTR_ERR(devconf1);
+
+    return 0;
+}
+
+struct platform_driver omap_ctrl_driver = {
+    .probe = omap_ctrl_probe,
+    .driver = {
+    .name = "omap-ctrl",
+    .owner = THIS_MODULE,
+    },
+};
+
+static int __init omap_ctrl_init(void)
+{
+    return platform_driver_register(&omap_ctrl_driver);
+}
+subsys_initcall(omap_ctrl_init);
+
+static void __exit omap_ctrl_exit(void)
+{
+    platform_driver_unregister(&omap_ctrl_driver);
+}

I use that driver with platform device in board file with proper
resources and it works fine. It could be easily added also DT
bindings.
Also you mention somewhere in drivers (but I think misc isn't right
place for that). Any opinion on that? Thanks for feedback.

>
> The reason why it should be a separate driver is that the system
> control module can live a separate runtime PM life from the
> drivers using the CONTROL_DEVCONF register bits.
>
> Regards,
>
> Tony

BR,

marek

-- 
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer

Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
twitter: #opennandra
web: http://open-nandra.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists