* [GIT PULL] fbdev fixes for 3.18
From: Tomi Valkeinen @ 2014-10-30 13:20 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Linux Fbdev development list, LKML
[-- Attachment #1: Type: text/plain, Size: 3739 bytes --]
Hi Linus,
Please pull fbdev fixes for 3.18.
Tomi
The following changes since commit f114040e3ea6e07372334ade75d1ee0775c355e1:
Linux 3.18-rc1 (2014-10-19 18:08:38 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git tags/fbdev-fixes-3.18
for you to fetch changes up to a942535d6e5c23f0015d6674d28c91dd59a720e4:
Merge branch '3.18/omapdss-fixes' into 3.18/fbdev-fixes (2014-10-30 14:53:49 +0200)
----------------------------------------------------------------
fbdev fixes for 3.18
* Fix fb console option parsing
* Fixes for OMAPDSS/OMAPFB crashes related to module unloading and
device/driver binding & unbinding.
* Fix for OMAP HDMI PLL locking failing in certain cases
* Misc minor fixes for atmel lcdfb and OMAP
----------------------------------------------------------------
Alexandre Belloni (1):
drivers: video: fbdev: atmel_lcdfb.c: remove unnecessary header
Felipe Balbi (2):
OMAPFB: add missing MODULE_ALIAS()
OMAPFB: remove __exit annotation
Maarten ter Huurne (1):
fbcon: Fix option parsing control flow in fb_console_setup
Marek Belisko (1):
omap: dss: connector-analog-tv: Add missing module device table
Mark Rustad (1):
video/console: Resolve several shadow warnings
Tomi Valkeinen (10):
OMAPDSS: set suppress_bind_attrs
OMAPDSS: apply: wait pending updates on manager disable
OMAPFB: fix overlay disable when freeing resources.
OMAPFB: fix releasing overlays
OMAPDSS: HDMI: fix PLL GO bit handling
OMAPDSS: HDMI: fix regsd write
OMAPDSS: DISPC: fix mflag offset
OMAPDSS: fix dispc register dump for preload & mflag
OMAPDSS: DSI: Fix PLL_SELFEQDCO field width
Merge branch '3.18/omapdss-fixes' into 3.18/fbdev-fixes
drivers/video/console/fbcon.c | 19 ++++++++---------
drivers/video/console/vgacon.c | 24 +++++++++++-----------
drivers/video/fbdev/atmel_lcdfb.c | 1 -
.../fbdev/omap2/displays-new/connector-analog-tv.c | 3 +++
.../video/fbdev/omap2/displays-new/connector-dvi.c | 1 +
.../fbdev/omap2/displays-new/connector-hdmi.c | 1 +
.../fbdev/omap2/displays-new/encoder-tfp410.c | 1 +
.../fbdev/omap2/displays-new/encoder-tpd12s015.c | 1 +
drivers/video/fbdev/omap2/displays-new/panel-dpi.c | 1 +
.../video/fbdev/omap2/displays-new/panel-dsi-cm.c | 1 +
.../omap2/displays-new/panel-lgphilips-lb035q02.c | 1 +
.../omap2/displays-new/panel-nec-nl8048hl11.c | 1 +
.../omap2/displays-new/panel-sharp-ls037v7dw01.c | 1 +
.../omap2/displays-new/panel-sony-acx565akm.c | 1 +
.../omap2/displays-new/panel-tpo-td028ttec1.c | 1 +
.../omap2/displays-new/panel-tpo-td043mtea1.c | 1 +
drivers/video/fbdev/omap2/dss/apply.c | 2 ++
drivers/video/fbdev/omap2/dss/dispc.c | 8 ++++----
drivers/video/fbdev/omap2/dss/dispc.h | 3 +--
drivers/video/fbdev/omap2/dss/dpi.c | 1 +
drivers/video/fbdev/omap2/dss/dsi.c | 3 ++-
drivers/video/fbdev/omap2/dss/dss.c | 1 +
drivers/video/fbdev/omap2/dss/hdmi4.c | 1 +
drivers/video/fbdev/omap2/dss/hdmi5.c | 1 +
drivers/video/fbdev/omap2/dss/hdmi_pll.c | 13 ++++++------
drivers/video/fbdev/omap2/dss/rfbi.c | 1 +
drivers/video/fbdev/omap2/dss/sdi.c | 1 +
drivers/video/fbdev/omap2/dss/venc.c | 1 +
drivers/video/fbdev/omap2/omapfb/omapfb-main.c | 18 ++++++++--------
29 files changed, 67 insertions(+), 46 deletions(-)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] omap: dss: connector-analog-tv: Add missing module device table
From: Tomi Valkeinen @ 2014-10-30 12:52 UTC (permalink / raw)
To: Marek Belisko; +Cc: plagnioj, hns, linux-omap, linux-fbdev, linux-kernel
In-Reply-To: <1414441443-10224-1-git-send-email-marek@goldelico.com>
[-- Attachment #1: Type: text/plain, Size: 940 bytes --]
On 27/10/14 22:24, Marek Belisko wrote:
> Without that fix connector-analog-tv driver isn't probed when compiled
> as module.
>
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
> drivers/video/fbdev/omap2/displays-new/connector-analog-tv.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/video/fbdev/omap2/displays-new/connector-analog-tv.c b/drivers/video/fbdev/omap2/displays-new/connector-analog-tv.c
> index 5ee3b55..d29cf72 100644
> --- a/drivers/video/fbdev/omap2/displays-new/connector-analog-tv.c
> +++ b/drivers/video/fbdev/omap2/displays-new/connector-analog-tv.c
> @@ -301,6 +301,8 @@ static const struct of_device_id tvc_of_match[] = {
> {},
> };
>
> +MODULE_DEVICE_TABLE(of, tvc_of_match);
> +
> static struct platform_driver tvc_connector_driver = {
> .probe = tvc_probe,
> .remove = __exit_p(tvc_remove),
>
Thanks, queued for 3.18 fixes.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 0/5] arm: sa1100: fix sa1100 fb and pcmcia w/o cpufreq
From: Dmitry Eremin-Solenikov @ 2014-10-29 18:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1414147047-12892-1-git-send-email-dbaryshkov@gmail.com>
2014-10-24 13:37 GMT+03:00 Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>:
> Hello,
>
> These patches were sitting in my queue for some time. On SA-1100
> framebuffer and PCMCIA drivers make use of cpufreq_get(0) function call
> to determine the cpu frequency. Russell's commit
> 1937f5b91833e2e8e53bcc821fc7a5fbe6ccb9b5 (ARM: fix sa1100 build) fixed
> the build issues, but broke two devices (Collie and Jornada720). For
> those two boards the cpufreq code gets compiled but is not enabled (as
> board files do not provide timing information for the CPUFREQ driver).
> Thus cpufreq_get(0) returns incorrect values and incorrect timings get
> programmed into the hardware.
>
> PXA2xx (the very similar platform) uses Clock API to determine CPU
> frequency both in framebuffer and PCMCIA drivers. These patches make
> similar changes to StrongARM drivers.
>
> These patches are required to make use of framebuffer and CF card on
> Sharp Collie (and possibly on HP Jornada 720).
Russell, what about these patches?
They are absolutely required for Collie to work properly.
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH v4 0/5] simplefb: add clock handling code
From: Tomi Valkeinen @ 2014-10-29 11:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <544F7E5D.60104@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 2477 bytes --]
Hi Hans, Rob,
On 28/10/14 13:30, Hans de Goede wrote:
> Hi,
>
> On 10/28/2014 12:11 PM, Rob Herring wrote:
>> Yes, I object to the binding still as it has not changed from what was
>> previously posted.
>
> It would be helpful if you could explain why you object. Last time you
> said: " You are mixing in a hardware description that is simply inaccurate."
>
> I then explained that this is not hardware description, but runtime state
> information, as it tells the kernel which clocks were chosen to drive the
> display (out of typically a list of possible options, depending on which
> output is used, etc.). Just like which memory address the bootloader has
> chosen to scan out the video image from.
>
> Then you got quiet, so sorry, but this time your objection really is too
> late. You cannot simply go quiet halfway through a discussion and then pop
> up again when a new version is posted to say "I object" yet another time,
> you've had your chance to make your arguments last time, and chose to stay
> quiet after I explained in detail that this is not hardware description but
> state information, so now it is simply too late.
>
> These bindings have been discussed at Plumbers with various interested people
> present, and the conclusion was that this really is the best way to handle this,
> so this patch is:
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Mike Turquette <mturquette@linaro.org>
> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Reviewed-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>
> And David Herrman who is working on simpledrm, which will be merged soon, which
> will also use the simplefb bindings also agrees. So we have the simplefb maintainer,
> simpledrm maintainer, and the clk subsystem maintainer + 2 other maintainers all
> agreeing on a way forward, the time for bikeshedding now really really really is
> over.
>
> Tomi, can you please let us know how you plan to proceed with this ?
I won't merge DT bindings via fbdev tree, if a DT maintainer says no.
I took Rob's silence to the earlier series as a silent ack for your
explanation. Obviously that was not the case.
Rob, please advice asap what should be done to the bindings to get your
ack. As Hans explained above, this discussion has been going on for a
long time, and afaik this series is the best way forward of all the
options discussed.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] drivers: depend on instead of select BACKLIGHT_CLASS_DEVICE and ACPI_VIDEO
From: Michael Ellerman @ 2014-10-29 8:27 UTC (permalink / raw)
To: Jani Nikula
Cc: Randy Dunlap, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
linux-fbdev, Jingoo Han, Daniel Drake, Jens Frederich,
David Airlie, Greg Kroah-Hartman, Jon Nettleton, linux-usb,
linux-kernel, dri-devel, platform-driver-x86, Lee Jones,
Laurent Pinchart, Darren Hart, Daniel Vetter, Bryan Wu,
linuxppc-dev
In-Reply-To: <87tx2njooz.fsf@intel.com>
On Wed, 2014-10-29 at 09:54 +0200, Jani Nikula wrote:
> On Wed, 29 Oct 2014, Michael Ellerman <mpe@ellerman.id.au> wrote:
> > On Tue, 2014-10-28 at 13:29 -0700, Randy Dunlap wrote:
> >> On 10/27/14 06:13, Tomi Valkeinen wrote:
> >> > I also think the 'depends on BACKLIGHT_CLASS_DEVICE ||
> >> > BACKLIGHT_CLASS_DEVICE=n' pattern is quite... interesting (i.e. sounds
> >> > like a hack to me =).
> >>
> >> It does exactly what is needed and it is used in many places in kernel
> >> Kconfig files.
> >
> > Is there any reason you can't do:
> >
> > depends on BACKLIGHT_CLASS_DEVICE != m
>
> That's not the same thing. The FOO || FOO=n allows for all options, but
> forbids it being a module when the option depending on it is
> built-in.
OK right. Because "BAR depends on FOO" is short for "depends on FOO=y || FOO=m",
but also adds the implicit condition that if FOO=m then BAR must also be m.
Thanks for clueing me in.
cheers
^ permalink raw reply
* Re: [PATCH] drivers: depend on instead of select BACKLIGHT_CLASS_DEVICE and ACPI_VIDEO
From: Jani Nikula @ 2014-10-29 7:54 UTC (permalink / raw)
To: Michael Ellerman, Randy Dunlap
Cc: linux-fbdev, linux-usb, Daniel Drake, Jens Frederich,
Greg Kroah-Hartman, Jon Nettleton, Jingoo Han, linux-kernel,
dri-devel, platform-driver-x86, Bryan Wu, linuxppc-dev,
Tomi Valkeinen, Laurent Pinchart, Daniel Vetter, Darren Hart,
Lee Jones, Jean-Christophe Plagniol-Villard
In-Reply-To: <1414551853.7417.1.camel@concordia>
On Wed, 29 Oct 2014, Michael Ellerman <mpe@ellerman.id.au> wrote:
> On Tue, 2014-10-28 at 13:29 -0700, Randy Dunlap wrote:
>> On 10/27/14 06:13, Tomi Valkeinen wrote:
>> > I also think the 'depends on BACKLIGHT_CLASS_DEVICE ||
>> > BACKLIGHT_CLASS_DEVICE=n' pattern is quite... interesting (i.e. sounds
>> > like a hack to me =).
>>
>> It does exactly what is needed and it is used in many places in kernel
>> Kconfig files.
>
> Is there any reason you can't do:
>
> depends on BACKLIGHT_CLASS_DEVICE != m
That's not the same thing. The FOO || FOO=n allows for all options, but
forbids it being a module when the option depending on it is
built-in. Obviously something that's built-in can't depend on something
built as a module.
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
^ permalink raw reply
* Re: [PATCH] drivers: depend on instead of select BACKLIGHT_CLASS_DEVICE and ACPI_VIDEO
From: Michael Ellerman @ 2014-10-29 3:04 UTC (permalink / raw)
To: Randy Dunlap
Cc: Tomi Valkeinen, Jani Nikula, Jean-Christophe Plagniol-Villard,
linux-fbdev, Jingoo Han, Daniel Drake, Jens Frederich,
David Airlie, Greg Kroah-Hartman, Jon Nettleton, linux-usb,
linux-kernel, dri-devel, platform-driver-x86, Lee Jones,
Laurent Pinchart, Darren Hart, Daniel Vetter, Bryan Wu,
linuxppc-dev
In-Reply-To: <544FFC91.9040104@infradead.org>
On Tue, 2014-10-28 at 13:29 -0700, Randy Dunlap wrote:
> On 10/27/14 06:13, Tomi Valkeinen wrote:
> > I also think the 'depends on BACKLIGHT_CLASS_DEVICE ||
> > BACKLIGHT_CLASS_DEVICE=n' pattern is quite... interesting (i.e. sounds
> > like a hack to me =).
>
> It does exactly what is needed and it is used in many places in kernel
> Kconfig files.
Is there any reason you can't do:
depends on BACKLIGHT_CLASS_DEVICE != m
cheers
^ permalink raw reply
* Re: [PATCH 11/15] sound: soc: poodle: make use of new locomo GPIO interface
From: Alexandre Courbot @ 2014-10-29 3:03 UTC (permalink / raw)
To: Mark Brown
Cc: Dmitry Eremin-Solenikov, linux-arm-kernel@lists.infradead.org,
linux-gpio@vger.kernel.org, linux-input, linux-leds, linux-spi,
linux-fbdev@vger.kernel.org, alsa-devel@alsa-project.org,
Andrea Adami, Russell King, Daniel Mack, Haojian Zhuang,
Robert Jarzmik, Linus Walleij, Dmitry Torokhov, Bryan Wu,
Richard Purdie, Samuel Ortiz, Lee Jones, Jingoo Han,
Liam Girdwood
In-Reply-To: <20141028145850.GU18557@sirena.org.uk>
On Tue, Oct 28, 2014 at 11:58 PM, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Oct 28, 2014 at 03:02:04AM +0300, Dmitry Eremin-Solenikov wrote:
>> Since LoCoMo driver has been converted to provide proper gpiolib
>> interface, make poodle ASoC platform driver use gpiolib API.
>
> Please use subject lines matching the style for the subsystem.
>
>> + ret = gpio_request_array(poodle_gpios, ARRAY_SIZE(poodle_gpios));
>> + if (ret) {
>> + dev_err(&pdev->dev, "gpio_request_array() failed: %d\n",
>> + ret);
>> + return ret;
>> + }
>
> I sense a need for devm_gpio_request_array() here. Otherwise this looks
> fine - ideally it'd move to gpiod but moving to gpiolib is a clear win
> so no need to block on this.
I wish Dmitry took the opportunity to move this driver to the gpiod
API, especially since doing so would be trivial for this driver. Not a
critical requirement though, the present patch is already an
improvement. But if you want to do that last step, please have a look
at Documentation/gpio/consumer.txt and the "Platform Data" section of
Documentation/gpio/board.txt.
^ permalink raw reply
* Re: [PATCH] drivers: depend on instead of select BACKLIGHT_CLASS_DEVICE and ACPI_VIDEO
From: Randy Dunlap @ 2014-10-28 20:29 UTC (permalink / raw)
To: Tomi Valkeinen, Jani Nikula
Cc: linux-kernel, dri-devel, linuxppc-dev, platform-driver-x86,
linux-usb, linux-fbdev, David Airlie, Daniel Vetter,
Greg Kroah-Hartman, Darren Hart, Laurent Pinchart,
Benjamin Herrenschmidt, Jens Frederich, Daniel Drake,
Jon Nettleton, Jean-Christophe Plagniol-Villard, Jingoo Han,
Bryan Wu, Lee Jones
In-Reply-To: <544E44E9.4040208@ti.com>
On 10/27/14 06:13, Tomi Valkeinen wrote:
> On 27/10/14 13:59, Jani Nikula wrote:
>
>>> While doing 'depends on' instead of 'select' is an "easy" fix for this,
>>> I do dislike it quite a bit. It's a major pain to go around the kernel
>>> config, trying to find all the dependencies that a particular driver
>>> wants. If I need fb-foobar, I should just be able to enable it, instead
>>> of first searching and selecting its minor dependencies individually.
>>
>> Agreed, but I don't think that's specific to this patch.
>
> Well, no, the generic problem is not specific to this patch, but we can
> avoid the issue with proper use of 'select' (at least in some cases),
> which is specific to this patch.
>
>>> So, not a NACK, but a "isn't there an another way to fix this?".
>>
>> I think the real answer would be to fix kconfig to also show menu items
>> whose dependencies are not met, and then recursively enabling the
>> dependencies when the item is enabled. Beyond my scope.
>>
>>> Looking at backlight... BACKLIGHT_LCD_SUPPORT seems to be a "meta"
>>> option, it only enables a Kconfig submenu.
>>>
>>> So I think we could just remove the whole BACKLIGHT_LCD_SUPPORT option.
>>> But if we do that, all the items in drivers/video/backlight/Kconfig with
>>> default 'y' or 'm' would get enabled by default, so I think we should
>>> remove the 'default's from that file. That makes sense in any case, as I
>>> don't see why "HP Jornada 700 series LCD Driver" should be "default y".
>>>
>>> BACKLIGHT_CLASS_DEVICE doesn't depend on anything except
>>> BACKLIGHT_LCD_SUPPORT, so after removing BACKLIGHT_LCD_SUPPORT it should
>>> be safe to 'select' BACKLIGHT_CLASS_DEVICE.
>>>
>>> BACKLIGHT_CLASS_DEVICE could be made a hidden option, and the drivers in
>>> drivers/video/backlight/Kconfig which are under BACKLIGHT_CLASS_DEVICE
>>> could be made to select BACKLIGHT_CLASS_DEVICE instead.
>>
>> I think it should be possible to choose between y and m when it's
>
> If I'm not mistaken, if CONFIG_FOO is 'm', and it 'select's CONFIG_BAR,
> and CONFIG_BAR is tristate, then CONFIG_BAR will be set to 'm'.
>
>> selected, and it should be possible to enable it when it's not selected
>> by any drivers. I'm not sure a hidden option is good for that.
>
> Why would you want to enable it if no one uses it? Does
> BACKLIGHT_CLASS_DEVICE enable something even if no driver uses it?
>
>>> That doesn't exactly fix anything, but I think it makes sense as
>>> BACKLIGHT_CLASS_DEVICE is something that's selected from all around the
>>> kernel, so it should be a selectable "library" instead of a Kconfig menu
>>> option.
>>
>> At least for drm/i915 BACKLIGHT_CLASS_DEVICE is "an option". We use it
>> if it's enabled, but we are just fine if it's not. I've learned the way
>> to express that is
>>
>> depends on BACKLIGHT_CLASS_DEVICE || BACKLIGHT_CLASS_DEVICE=n
>>
>> but I don't think there's a way to express that in terms of select, is
>> there? The dependency above guarantees there's no DRM_I915=y and
>> BACKLIGHT_CLASS_DEVICE=m combo which would fail. And this, btw, is where
>> this whole patch got started, as select didn't handle that properly.
>
> If backlight support is considered an option for drm/i915, then I think
> there should be a Kconfig option for i915 to enable backlight support,
> which in turn selects BACKLIGHT_CLASS_DEVICE. And that select will force
> BACKLIGHT_CLASS_DEVICE to be built-in if drm/i915 is built-in.
>
> Oh, but it doesn't work optimally with modules. The new option needed
> for that would be boolean, so BACKLIGHT_CLASS_DEVICE would always be
> either y or n. Sigh...
>
>>> I didn't look at the ACPI_VIDEO side, so no idea how messy that is.
>>
>> Basically it's another dependency on BACKLIGHT_CLASS_DEVICE. I can only
>> imagine trying to solve this problem with select is going to end up in
>> recursive dependencies that spread out and need changing about as wide
>> as this patch.
>
> If ACPI_VIDEO uses select to enable BACKLIGHT_CLASS_DEVICE, then, I
> think, selecting ACPI_VIDEO will also select BACKLIGHT_CLASS_DEVICE. So
> I don't right away see any recursive dependencies. Or what did you have
> in mind?
>
>> In the end, I agree with the problem you have with this patch, but yet I
>> think it's the right thing to do in terms of expressing the
>> dependencies.
>
> Well, dri/i915 doesn't exactly depend on backlight, if I understood you
> correctly. Instead, backlight is an option for dri/i915, and you kind of
> hack it to be implemented with that 'depends on BACKLIGHT_CLASS_DEVICE
> || BACKLIGHT_CLASS_DEVICE=n'.
>
> I guess it's debatable whether drivers should automatically use features
> in the kernel if they happen to be enabled in the Kconfig, or should
> they be individually enabled for that driver. I personally like the
> latter option, as it allows more precise control, but it probably also
> depends on the feature in question.
>
> I also think the 'depends on BACKLIGHT_CLASS_DEVICE ||
> BACKLIGHT_CLASS_DEVICE=n' pattern is quite... interesting (i.e. sounds
> like a hack to me =).
It does exactly what is needed and it is used in many places in kernel
Kconfig files.
--
~Randy
^ permalink raw reply
* Re: [PATCH 12/15] ARM: pxa: poodle: don't preallocate IRQ space for locomo
From: Robert Jarzmik @ 2014-10-28 19:13 UTC (permalink / raw)
To: Dmitry Eremin-Solenikov
Cc: Alexandre Courbot, linux-fbdev, Bryan Wu, Russell King,
Samuel Ortiz, Liam Girdwood, Linus Walleij, Dmitry Torokhov,
alsa-devel, Haojian Zhuang, linux-spi, linux-gpio, Mark Brown,
Andrea Adami, linux-arm-kernel, linux-input, Jingoo Han,
Daniel Mack, Lee Jones, Richard Purdie, linux-leds
In-Reply-To: <1414454528-24240-13-git-send-email-dbaryshkov@gmail.com>
Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> writes:
...zip...
> diff --git a/arch/arm/mach-pxa/poodle.c b/arch/arm/mach-pxa/poodle.c
> index 3041d83..97eced9 100644
> --- a/arch/arm/mach-pxa/poodle.c
> +++ b/arch/arm/mach-pxa/poodle.c
> @@ -181,14 +181,18 @@ static struct locomo_platform_data locomo_info = {
> .gpio_data = {
> .gpio_base = POODLE_LOCOMO_GPIO_BASE,
> },
> - .bl_data = {
> + .lcd_data = {
Is that part really belonging to that patch ? It seems to me that this changes
the code included in patch 10/15 "ARM: pxa: poodle: use new LoCoMo
driver".
Therefore I'm wondering if I take the serie, halt it at patch 10, and try to
compile I would succeed ... I think I wouldn't.
> + },
> + .bl_data = {
> .gpio_fl_vr = POODLE_GPIO_FL_VR,
> },
Ditto.
> + .gpio_amp1_on = -1,
> + .gpio_amp2_on = -1,
This won't prevent compilation, but is it the target of this patch which is IRQ
space oriented ?
> @@ -453,7 +457,7 @@ static void __init poodle_init(void)
> platform_scoop_config = &poodle_pcmcia_config;
>
> if (sharpsl_param.comadj != -1)
> - locomo_info.bl_data.comadj = sharpsl_param.comadj;
> + locomo_info.lcd_data.comadj = sharpsl_param.comadj;
Ditto.
Cheers.
--
Robert
^ permalink raw reply
* Re: [PATCH 06/15] video: lcd: add LoCoMo LCD driver
From: Dmitry Eremin-Solenikov @ 2014-10-28 16:47 UTC (permalink / raw)
To: Jingoo Han
Cc: linux-arm-kernel, linux-gpio, linux-input, linux-leds, linux-spi,
linux-fbdev, alsa-devel, 'Andrea Adami',
'Russell King', 'Daniel Mack',
'Haojian Zhuang', 'Robert Jarzmik',
'Linus Walleij', 'Alexandre Courbot',
'Dmitry Torokhov', 'Bryan Wu',
'Richard Purdie', 'Samuel Ortiz',
'Lee Jones', 'Mark Brown',
'Liam Girdwood'
In-Reply-To: <000101cff246$6d229850$4767c8f0$%han@samsung.com>
On 10/28/2014 03:30 AM, Jingoo Han wrote:
> On Tuesday, October 28, 2014 9:02 AM, Dmitry Eremin-Solenikov wrote:
>>
>> LoCoMo has some special handling for TFT screens attached to Collie and
>> Poodle. Implement that as a separate driver.
Thanks for the review, changes for both LCD and Backlight will be
implemented in V2.
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH 11/15] sound: soc: poodle: make use of new locomo GPIO interface
From: Dmitry Eremin-Solenikov @ 2014-10-28 16:45 UTC (permalink / raw)
To: Mark Brown
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-gpio-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-leds-u79uwXL29TY76Z2rM5mHXA,
linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Andrea Adami, Russell King,
Daniel Mack, Haojian Zhuang, Robert Jarzmik, Linus Walleij,
Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Richard Purdie,
Samuel Ortiz, Lee Jones, Jingoo Han, Liam Girdwood
In-Reply-To: <20141028145850.GU18557-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
On 10/28/2014 05:58 PM, Mark Brown wrote:
> On Tue, Oct 28, 2014 at 03:02:04AM +0300, Dmitry Eremin-Solenikov wrote:
>> Since LoCoMo driver has been converted to provide proper gpiolib
>> interface, make poodle ASoC platform driver use gpiolib API.
>
> Please use subject lines matching the style for the subsystem.
>
>> + ret = gpio_request_array(poodle_gpios, ARRAY_SIZE(poodle_gpios));
>> + if (ret) {
>> + dev_err(&pdev->dev, "gpio_request_array() failed: %d\n",
>> + ret);
>> + return ret;
>> + }
>
> I sense a need for devm_gpio_request_array() here. Otherwise this looks
> fine - ideally it'd move to gpiod but moving to gpiolib is a clear win
> so no need to block on this.
I like the idea of devm_gpio_request_array. But I would like not to add
additional (core) patches to this patchset.
>
> Acked-by: Mark Brown <broonie@kernel.org>
>
> with at least the subject line fixed.
Subject line fixed.
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH 11/15] sound: soc: poodle: make use of new locomo GPIO interface
From: Mark Brown @ 2014-10-28 14:58 UTC (permalink / raw)
To: Dmitry Eremin-Solenikov
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-gpio-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-leds-u79uwXL29TY76Z2rM5mHXA,
linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Andrea Adami, Russell King,
Daniel Mack, Haojian Zhuang, Robert Jarzmik, Linus Walleij,
Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Richard Purdie,
Samuel Ortiz, Lee Jones, Jingoo Han, Liam Girdwood
In-Reply-To: <1414454528-24240-12-git-send-email-dbaryshkov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 707 bytes --]
On Tue, Oct 28, 2014 at 03:02:04AM +0300, Dmitry Eremin-Solenikov wrote:
> Since LoCoMo driver has been converted to provide proper gpiolib
> interface, make poodle ASoC platform driver use gpiolib API.
Please use subject lines matching the style for the subsystem.
> + ret = gpio_request_array(poodle_gpios, ARRAY_SIZE(poodle_gpios));
> + if (ret) {
> + dev_err(&pdev->dev, "gpio_request_array() failed: %d\n",
> + ret);
> + return ret;
> + }
I sense a need for devm_gpio_request_array() here. Otherwise this looks
fine - ideally it'd move to gpiod but moving to gpiolib is a clear win
so no need to block on this.
Acked-by: Mark Brown <broonie@kernel.org>
with at least the subject line fixed.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH v4 0/5] simplefb: add clock handling code
From: Hans de Goede @ 2014-10-28 11:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAL_JsqJ2JRg7k21CEVe4-At7enD+y3TN_7wfqhY0OHZm-_j17w@mail.gmail.com>
Hi,
On 10/28/2014 12:11 PM, Rob Herring wrote:
> On Tue, Oct 28, 2014 at 6:44 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> On 22/10/14 19:45, Hans de Goede wrote:
>>
>>> Since we seem to have broad agreement on how to move forward with this, I
>>> would like to ask you to please merge this patch-set for 3.19.
>>>
>>> I know 3.19 is still somewhat ar away, but I would like to submit the u-boot
>>> side of this to upstream ASAP, so can you please let me know if you plan to
>>> take this patch-set for 3.19 soon ?
>>
>> This series looks fine to me, except patch 3 is missing a description,
>> and patch 4 makes the desc a continuation of the subject. Each patch
>> should have a description (independent from subject).
>>
>> No need to resend, I can cook up something for those, or I can edit the
>> descs if you provide the text.
>>
>> So if there are no strong objections, I'll queue this for 3.19.
>
> Yes, I object to the binding still as it has not changed from what was
> previously posted.
It would be helpful if you could explain why you object. Last time you
said: " You are mixing in a hardware description that is simply inaccurate."
I then explained that this is not hardware description, but runtime state
information, as it tells the kernel which clocks were chosen to drive the
display (out of typically a list of possible options, depending on which
output is used, etc.). Just like which memory address the bootloader has
chosen to scan out the video image from.
Then you got quiet, so sorry, but this time your objection really is too
late. You cannot simply go quiet halfway through a discussion and then pop
up again when a new version is posted to say "I object" yet another time,
you've had your chance to make your arguments last time, and chose to stay
quiet after I explained in detail that this is not hardware description but
state information, so now it is simply too late.
These bindings have been discussed at Plumbers with various interested people
present, and the conclusion was that this really is the best way to handle this,
so this patch is:
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Mike Turquette <mturquette@linaro.org>
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
Reviewed-by: Maxime Ripard <maxime.ripard@free-electrons.com>
And David Herrman who is working on simpledrm, which will be merged soon, which
will also use the simplefb bindings also agrees. So we have the simplefb maintainer,
simpledrm maintainer, and the clk subsystem maintainer + 2 other maintainers all
agreeing on a way forward, the time for bikeshedding now really really really is
over.
Tomi, can you please let us know how you plan to proceed with this ?
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v4 0/5] simplefb: add clock handling code
From: Rob Herring @ 2014-10-28 11:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <544F737A.7000109@ti.com>
On Tue, Oct 28, 2014 at 6:44 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On 22/10/14 19:45, Hans de Goede wrote:
>
>> Since we seem to have broad agreement on how to move forward with this, I
>> would like to ask you to please merge this patch-set for 3.19.
>>
>> I know 3.19 is still somewhat ar away, but I would like to submit the u-boot
>> side of this to upstream ASAP, so can you please let me know if you plan to
>> take this patch-set for 3.19 soon ?
>
> This series looks fine to me, except patch 3 is missing a description,
> and patch 4 makes the desc a continuation of the subject. Each patch
> should have a description (independent from subject).
>
> No need to resend, I can cook up something for those, or I can edit the
> descs if you provide the text.
>
> So if there are no strong objections, I'll queue this for 3.19.
Yes, I object to the binding still as it has not changed from what was
previously posted.
Rob
^ permalink raw reply
* Re: [PATCH v4 0/5] simplefb: add clock handling code
From: Hans de Goede @ 2014-10-28 11:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <544F737A.7000109@ti.com>
Hi,
On 10/28/2014 11:44 AM, Tomi Valkeinen wrote:
> On 22/10/14 19:45, Hans de Goede wrote:
>
>> Since we seem to have broad agreement on how to move forward with this, I
>> would like to ask you to please merge this patch-set for 3.19.
>>
>> I know 3.19 is still somewhat ar away, but I would like to submit the u-boot
>> side of this to upstream ASAP, so can you please let me know if you plan to
>> take this patch-set for 3.19 soon ?
>
> This series looks fine to me, except patch 3 is missing a description,
Description for patch 3:
"Add a proper struct describing simplefb private data, with the palette in there,
instead of directly storing the palette in the fb_info->par pointer."
> and patch 4 makes the desc a continuation of the subject.
Description for patch 4:
"Use the usual kernel style of goto error_foo to free resources on probe
error. This is a preparation patch for adding clocks support.
While at it update ioremap_wc error return from ENODEV to ENOMEM."
> Each patch should have a description (independent from subject).
Ok, I'll remember that for future patches.
> No need to resend, I can cook up something for those, or I can edit the
> descs if you provide the text.
Descriptions provided above, thanks!
> So if there are no strong objections, I'll queue this for 3.19.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH 15/15] spi: add locomo SPI driver
From: Mark Brown @ 2014-10-28 11:03 UTC (permalink / raw)
To: Dmitry Eremin-Solenikov
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-gpio-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-leds-u79uwXL29TY76Z2rM5mHXA,
linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Andrea Adami, Russell King,
Daniel Mack, Haojian Zhuang, Robert Jarzmik, Linus Walleij,
Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Richard Purdie,
Samuel Ortiz, Lee Jones, Jingoo Han, Liam Girdwood
In-Reply-To: <1414454528-24240-16-git-send-email-dbaryshkov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1505 bytes --]
On Tue, Oct 28, 2014 at 03:02:08AM +0300, Dmitry Eremin-Solenikov wrote:
> LoCoMo chip has a built-in simple SPI controller. On Sharp SL-5500 PDDAs
> it is connected to external MMC slot.
> +config SPI_LOCOMO
> + tristate "Locomo SPI master"
> + depends on MFD_LOCOMO
> + select SPI_BITBANG
Rather than using SPI_BITBANG it'd be good for new drivers to convert to
using the core transfer_one() functionality which replaces most of what
the bitbang code is doing. The bitbang functionality was misnamed for
most of the users and we're going to try to move most of the functionality
not actually related to bitbanging out of it.
> + /* if (locomospi_carddetect()) { */
> + r = readw(spidev->base + LOCOMO_SPIMD);
> + r |= LOCOMO_SPIMD_XON;
> + writew(r, spidev->base + LOCOMO_SPIMD);
> +
> + r = readw(spidev->base + LOCOMO_SPIMD);
> + r |= LOCOMO_SPIMD_XEN;
> + writew(r, spidev->base + LOCOMO_SPIMD);
> + /* } */
Either remove or implement the comments.
> + r = readw(spidev->base + LOCOMO_SPICT);
> + r |= LOCOMO_SPIMD_XEN; /* FIXME */
> + writew(r, spidev->base + LOCOMO_SPICT);
FIXME?
> + if (t)
> + hz = t->speed_hz;
> + if (!hz)
> + hz = spi->max_speed_hz;
The core will ensure that the transfer always has a speed set in it.
> +static int locomo_spi_probe(struct platform_device *pdev)
> +{
> + struct resource *res;
> + struct spi_master *master;
> + struct locomospi_dev *spidev;
> + int ret;
> +
> + dev_info(&pdev->dev, "LoCoO SPI Driver\n");
Remove this, it's not adding anything.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [PATCH 3/5] fbdev: sa1100fb: make use of device clock
From: Tomi Valkeinen @ 2014-10-28 10:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1414147047-12892-4-git-send-email-dbaryshkov@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 564 bytes --]
On 24/10/14 13:37, Dmitry Eremin-Solenikov wrote:
> Use per-device clock (instead of calling cpufreq_get(0), which can
> return 0 if no cpu frequency driver is selected) to program timings.
>
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> ---
> drivers/video/fbdev/sa1100fb.c | 24 +++++++++++++++++-------
> drivers/video/fbdev/sa1100fb.h | 1 +
> 2 files changed, 18 insertions(+), 7 deletions(-)
I presume this has to be merged with the rest of the patches, so:
Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v4 0/5] simplefb: add clock handling code
From: Tomi Valkeinen @ 2014-10-28 10:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1413996311-4287-1-git-send-email-hdegoede@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 775 bytes --]
On 22/10/14 19:45, Hans de Goede wrote:
> Since we seem to have broad agreement on how to move forward with this, I
> would like to ask you to please merge this patch-set for 3.19.
>
> I know 3.19 is still somewhat ar away, but I would like to submit the u-boot
> side of this to upstream ASAP, so can you please let me know if you plan to
> take this patch-set for 3.19 soon ?
This series looks fine to me, except patch 3 is missing a description,
and patch 4 makes the desc a continuation of the subject. Each patch
should have a description (independent from subject).
No need to resend, I can cook up something for those, or I can edit the
descs if you provide the text.
So if there are no strong objections, I'll queue this for 3.19.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 06/15] video: lcd: add LoCoMo LCD driver
From: Jingoo Han @ 2014-10-28 0:30 UTC (permalink / raw)
To: 'Dmitry Eremin-Solenikov'
Cc: linux-arm-kernel, linux-gpio, linux-input, linux-leds, linux-spi,
linux-fbdev, alsa-devel, 'Andrea Adami',
'Russell King', 'Daniel Mack',
'Haojian Zhuang', 'Robert Jarzmik',
'Linus Walleij', 'Alexandre Courbot',
'Dmitry Torokhov', 'Bryan Wu',
'Richard Purdie', 'Samuel Ortiz',
'Lee Jones', 'Mark Brown',
'Liam Girdwood', 'Jingoo Han'
In-Reply-To: <1414454528-24240-7-git-send-email-dbaryshkov@gmail.com>
On Tuesday, October 28, 2014 9:02 AM, Dmitry Eremin-Solenikov wrote:
>
> LoCoMo has some special handling for TFT screens attached to Collie and
> Poodle. Implement that as a separate driver.
>
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> ---
> drivers/video/backlight/Kconfig | 8 ++
> drivers/video/backlight/Makefile | 1 +
> drivers/video/backlight/locomo_lcd.c | 224 +++++++++++++++++++++++++++++++++++
> 3 files changed, 233 insertions(+)
> create mode 100644 drivers/video/backlight/locomo_lcd.c
>
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index 03b77b33..bc5c671 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -48,6 +48,14 @@ config LCD_LMS283GF05
> SPI driver for Samsung LMS283GF05. This provides basic support
> for powering the LCD up/down through a sysfs interface.
>
> +config LCD_LOCOMO
> + tristate "Sharp LOCOMO LCD Driver"
> + depends on MFD_LOCOMO
> + default y
> + help
> + If you have a Sharp Zaurus SL-5500 (Collie) or SL-5600 (Poodle) say y to
> + enable the LCD driver.
> +
> config LCD_LTV350QV
> tristate "Samsung LTV350QV LCD Panel"
> depends on SPI_MASTER
> diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> index 2a61b7e..b2580e7 100644
> --- a/drivers/video/backlight/Makefile
> +++ b/drivers/video/backlight/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_LCD_ILI922X) += ili922x.o
> obj-$(CONFIG_LCD_ILI9320) += ili9320.o
> obj-$(CONFIG_LCD_L4F00242T03) += l4f00242t03.o
> obj-$(CONFIG_LCD_LD9040) += ld9040.o
> +obj-$(CONFIG_LCD_LOCOMO) += locomo_lcd.o
Please insert this alphabetically for the readability.
> obj-$(CONFIG_LCD_LMS283GF05) += lms283gf05.o
> obj-$(CONFIG_LCD_LMS501KF03) += lms501kf03.o
> obj-$(CONFIG_LCD_LTV350QV) += ltv350qv.o
> diff --git a/drivers/video/backlight/locomo_lcd.c b/drivers/video/backlight/locomo_lcd.c
> new file mode 100644
> index 0000000..245efb8
> --- /dev/null
> +++ b/drivers/video/backlight/locomo_lcd.c
> @@ -0,0 +1,224 @@
> +/*
> + * Backlight control code for Sharp Zaurus SL-5500
> + *
> + * Copyright 2005 John Lenz <lenz@cs.wisc.edu>
> + * Maintainer: Pavel Machek <pavel@ucw.cz> (unless John wants to :-)
> + * GPL v2
> + *
> + * This driver assumes single CPU. That's okay, because collie is
> + * slightly old hardware, and no one is going to retrofit second CPU to
> + * old PDA.
> + */
> +
> +/* LCD power functions */
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/fb.h>
> +#include <linux/backlight.h>
> +#include <linux/err.h>
> +#include <linux/gpio.h>
> +#include <linux/delay.h>
> +#include <linux/lcd.h>
> +#include <linux/mfd/locomo.h>
Please, re-order these headers alphabetically.
It enhances the readability.
> +
> +static struct platform_device *locomolcd_dev;
> +static struct locomo_lcd_platform_data lcd_data;
> +static bool locomolcd_is_on;
> +static bool locomolcd_is_suspended;
> +static void __iomem *locomolcd_regs;
> +static struct lcd_device *lcd_dev;
> +
> +static struct gpio locomo_gpios[] = {
> + { 0, GPIOF_OUT_INIT_LOW, "LCD VSHA on" },
> + { 0, GPIOF_OUT_INIT_LOW, "LCD VSHD on" },
> + { 0, GPIOF_OUT_INIT_LOW, "LCD Vee on" },
> + { 0, GPIOF_OUT_INIT_LOW, "LCD MOD" },
> +};
> +
> +static void locomolcd_on(void)
> +{
> + gpio_set_value(lcd_data.gpio_lcd_vsha_on, 1);
> + mdelay(2);
> +
> + gpio_set_value(lcd_data.gpio_lcd_vshd_on, 1);
> + mdelay(2);
> +
> + locomo_m62332_senddata(locomolcd_dev->dev.parent, lcd_data.comadj, 0);
> + mdelay(5);
> +
> + gpio_set_value(lcd_data.gpio_lcd_vee_on, 1);
> + mdelay(10);
How about changing mdelay() to usleep_range()?
> +
> + /* TFTCRST | CPSOUT=0 | CPSEN */
> + writew(0x01, locomolcd_regs + LOCOMO_TC);
> +
> + /* Set CPSD */
> + writew(6, locomolcd_regs + LOCOMO_CPSD);
> +
> + /* TFTCRST | CPSOUT=0 | CPSEN */
> + writew((0x04 | 0x01), locomolcd_regs + LOCOMO_TC);
> + mdelay(10);
> +
> + gpio_set_value(lcd_data.gpio_lcd_mod, 1);
> +}
> +
> +static void locomolcd_off(void)
> +{
> + /* TFTCRST=1 | CPSOUT=1 | CPSEN = 0 */
> + writew(0x06, locomolcd_regs + LOCOMO_TC);
> + mdelay(1);
> +
> + gpio_set_value(lcd_data.gpio_lcd_vsha_on, 0);
> + mdelay(110);
> +
> + gpio_set_value(lcd_data.gpio_lcd_vee_on, 0);
> + mdelay(700);
> +
> + locomo_m62332_senddata(locomolcd_dev->dev.parent, 0, 0);
> + mdelay(5);
How about changing mdelay() to usleep_range() or msleep()?
> +
> + /* TFTCRST=0 | CPSOUT=0 | CPSEN = 0 */
> + writew(0, locomolcd_regs + LOCOMO_TC);
> + gpio_set_value(lcd_data.gpio_lcd_mod, 0);
> + gpio_set_value(lcd_data.gpio_lcd_vshd_on, 0);
> +}
> +
> +int locomo_lcd_set_power(struct lcd_device *lcd, int power)
> +{
> + dev_dbg(&lcd->dev, "LCD power %d (is %d)\n", power, locomolcd_is_on);
> + if (power = 0 && !locomolcd_is_on) {
> + locomolcd_is_on = 1;
> + locomolcd_on();
> + }
> + if (power != 0 && locomolcd_is_on) {
> + locomolcd_is_on = 0;
> + locomolcd_off();
> + }
> + return 0;
> +}
> +
> +static int locomo_lcd_get_power(struct lcd_device *lcd)
> +{
> + return !locomolcd_is_on;
> +}
> +
> +static struct lcd_ops locomo_lcd_ops = {
> + .set_power = locomo_lcd_set_power,
> + .get_power = locomo_lcd_get_power,
> +};
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int locomolcd_suspend(struct device *dev)
> +{
> + locomolcd_is_suspended = true;
> + locomolcd_off();
> +
> + return 0;
> +}
> +
> +static int locomolcd_resume(struct device *dev)
> +{
> + locomolcd_is_suspended = false;
> +
> + if (locomolcd_is_on)
> + locomolcd_on();
> +
> + return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(locomolcd_pm, locomolcd_suspend, locomolcd_resume);
> +#define LOCOMOLCD_PM (&locomolcd_pm)
> +#else
> +#define LOCOMOLCD_PM NULL
> +#endif
> +
> +static int locomolcd_probe(struct platform_device *dev)
> +{
> + unsigned long flags;
> + struct resource *res;
> + struct locomo_lcd_platform_data *pdata;
> + int rc;
> +
> + res = platform_get_resource(dev, IORESOURCE_MEM, 0);
> + if (!res)
> + return -EINVAL;
> + locomolcd_regs = devm_ioremap_resource(&dev->dev, res);
> + if (!locomolcd_regs)
> + return -EINVAL;
Please change it as below.
res = platform_get_resource(dev, IORESOURCE_MEM, 0);
locomolcd_regs = devm_ioremap_resource(&dev->dev, res);
if (IS_ERR(locomolcd_regs))
return PTR_ERR(locomolcd_regs);
> +
> + pdata = dev_get_platdata(&dev->dev);
> + if (!pdata)
> + return -EINVAL;
> +
> + lcd_data = *pdata;
> +
> + locomo_gpios[0].gpio = lcd_data.gpio_lcd_vsha_on;
> + locomo_gpios[1].gpio = lcd_data.gpio_lcd_vshd_on;
> + locomo_gpios[2].gpio = lcd_data.gpio_lcd_vee_on;
> + locomo_gpios[3].gpio = lcd_data.gpio_lcd_mod;
> + dev_info(&dev->dev, "GPIOs: %d %d %d %d\n",
> + locomo_gpios[0].gpio,
> + locomo_gpios[1].gpio,
> + locomo_gpios[2].gpio,
> + locomo_gpios[3].gpio);
> +
> + rc = gpio_request_array(locomo_gpios, ARRAY_SIZE(locomo_gpios));
> + if (rc)
> + return rc;
> +
> + local_irq_save(flags);
> + locomolcd_dev = dev;
> +
> + locomolcd_is_on = 1;
> + if (locomolcd_is_on)
> + locomolcd_on();
> +
> + local_irq_restore(flags);
> +
> + lcd_dev = lcd_device_register("locomo", &dev->dev, NULL,
Please use devm_lcd_device_register().
> + &locomo_lcd_ops);
> +
> + return 0;
> +}
> +
> +static int locomolcd_remove(struct platform_device *dev)
> +{
> + unsigned long flags;
> +
> + lcd_device_unregister(lcd_dev);
If devm_lcd_device_register() is used in probe(),
there is no need to call lcd_device_unregister() in remove().
> +
> + local_irq_save(flags);
> +
> + locomolcd_off();
> + locomolcd_dev = NULL;
> +
> + local_irq_restore(flags);
> +
> + gpio_free_array(locomo_gpios, ARRAY_SIZE(locomo_gpios));
> +
> + return 0;
> +}
> +
> +static void locomolcd_shutdown(struct platform_device *dev)
> +{
> + locomolcd_off();
> +}
> +
> +static struct platform_driver locomolcd_driver = {
> + .driver = {
> + .name = "locomo-lcd",
> + .owner = THIS_MODULE,
> + .pm = LOCOMOLCD_PM,
> + },
> + .probe = locomolcd_probe,
> + .remove = locomolcd_remove,
> + .shutdown = locomolcd_shutdown,
> +};
> +
> +module_platform_driver(locomolcd_driver);
> +
> +MODULE_AUTHOR("John Lenz <lenz@cs.wisc.edu>, Pavel Machek <pavel@ucw.cz>");
Please split these authors to lines as below.
MODULE_AUTHOR("John Lenz <lenz@cs.wisc.edu>");
MODULE_AUTHOR("Pavel Machek <pavel@ucw.cz>");
> +MODULE_DESCRIPTION("Collie LCD driver");
What does mean 'Collie'? 'Locomo' looks better.
> +MODULE_LICENSE("GPL");
How about using "GPL v2"?
Thank you.
Best regards,
Jingoo Han
> +MODULE_ALIAS("platform:locomo-lcd");
> --
> 2.1.1
^ permalink raw reply
* Re: [PATCH 00/15] new locomo driver
From: Mark Brown @ 2014-10-28 0:29 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Dmitry Eremin-Solenikov, Greg KH, linux-arm-kernel, linux-gpio,
linux-input, linux-leds, linux-spi, linux-fbdev, alsa-devel,
Andrea Adami, Daniel Mack, Haojian Zhuang, Robert Jarzmik,
Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu,
Richard Purdie, Samuel Ortiz, Lee Jones, Jingoo Han,
Liam Girdwood
In-Reply-To: <20141028001338.GZ27405@n2100.arm.linux.org.uk>
[-- Attachment #1: Type: text/plain, Size: 1825 bytes --]
On Tue, Oct 28, 2014 at 12:13:39AM +0000, Russell King - ARM Linux wrote:
> On Tue, Oct 28, 2014 at 03:01:53AM +0300, Dmitry Eremin-Solenikov wrote:
> > Sharp Zaurus SL-5500 and SL-5600 use special companion Gate Array. Current
> > drivers present in Linux kernel has some problems:
> > * It uses custom bus instead of platform bus/mfd core.
> I believe Greg wouldn't see that as a positive point.
> Don't think that the platform bus _should_ always be used. It shouldn't
> (Greg has said he'd like to see the platform bus to be totally killed off.)
> Instead, custom buses properly suited to the class of device in question
> is much preferred, especially if it aids in...
> > * Device drivers are not well layered/separated.
> ... better layering or separation of drivers.
> So, thinking that converting from a custom bus to a platform bus
> definitely is /not/ a positive step forward.
> (Why mfd was ever allowed to re-use the platform bus stuff is a separate
> question not relevent to these patches.)
The reason we ended up reusing the platform bus so much was that
originally people were doing custom buses but people started complaining
that the code was (or should be) a cut'n'paste of the platform bus and
that this duplication was both not pretty and a bit tedious for anyone
doing anything that involved deploying good practice over a lot of
buses. Early MFDs were actually MMIO devices so the platform bus was a
good fit, then people (including me when I upstreamed the wm97xx drivers
and apparently whoever worked on these devices) started adding custom
buses and then people complianed that we should reuse both the bus type
and the MFD infrastructure so here we are.
Probably a way of sharing the platform code but giving the bus another
name and bus_type would allow for better separation here.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [PATCH 00/15] new locomo driver
From: Dmitry Eremin-Solenikov @ 2014-10-28 0:28 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Greg KH, linux-arm-kernel,
linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-input,
linux-leds, linux-spi, linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Andrea Adami, Daniel Mack,
Haojian Zhuang, Robert Jarzmik, Linus Walleij, Alexandre Courbot,
Dmitry Torokhov, Bryan Wu, Richard Purdie, Samuel Ortiz,
Lee Jones, Mark Brown, Jingoo Han, Liam Girdwood
In-Reply-To: <20141028001338.GZ27405-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>
2014-10-28 4:13 GMT+04:00 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Tue, Oct 28, 2014 at 03:01:53AM +0300, Dmitry Eremin-Solenikov wrote:
>> Sharp Zaurus SL-5500 and SL-5600 use special companion Gate Array. Current
>> drivers present in Linux kernel has some problems:
>>
>> * It uses custom bus instead of platform bus/mfd core.
>
> I believe Greg wouldn't see that as a positive point.
>
> Don't think that the platform bus _should_ always be used. It shouldn't
> (Greg has said he'd like to see the platform bus to be totally killed off.)
> Instead, custom buses properly suited to the class of device in question
> is much preferred, especially if it aids in...
I'm sorry, I must be outdated on this topic. Few years ago things were exactly
opposite - we explicitly had to use platform bus for mfd devices. I
still think that
mfd devices containing several cells should use mfd-core (is it still
OK to use it?).
And mfd-core (up to now) uses platform bus.
>
>> * Device drivers are not well layered/separated.
>
> ... better layering or separation of drivers.
>
> So, thinking that converting from a custom bus to a platform bus
> definitely is /not/ a positive step forward.
Is there an example of new approach? Current locomo-dev implementation
is poisoned with several bugs. For example it passes ioremapped memory
as resources to child devices that later call request_resource() on it.
Adding additional features (like correct work with IRQs) will duplicate
platform bus code and mfd functions.
>
> (Why mfd was ever allowed to re-use the platform bus stuff is a separate
> question not relevent to these patches.)
I remember this story - Ian Molton and me wrote mfd-core.c.
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH 05/15] video: backlight: add new locomo backlight driver
From: Jingoo Han @ 2014-10-28 0:24 UTC (permalink / raw)
To: 'Dmitry Eremin-Solenikov'
Cc: 'Andrea Adami', 'Russell King',
'Daniel Mack', 'Haojian Zhuang',
'Robert Jarzmik', 'Linus Walleij',
linux-arm-kernel, linux-gpio, linux-input, linux-leds, linux-spi,
linux-fbdev, alsa-devel, 'Alexandre Courbot',
'Dmitry Torokhov', 'Bryan Wu',
'Richard Purdie', 'Samuel Ortiz',
'Lee Jones', 'Mark Brown',
'Liam Girdwood', 'Jingoo Han'
In-Reply-To: <1414454528-24240-6-git-send-email-dbaryshkov@gmail.com>
On Tuesday, October 28, 2014 9:02 AM, Dmitry Eremin-Solenikov wrote:
>
> Add new simple backlight driver - it cares only about PWM/frontlight
> part of LoCoMo, it does not touch TFT settings and does not export TFT
> power control.
>
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> ---
> drivers/video/backlight/Kconfig | 6 +-
> drivers/video/backlight/Makefile | 2 +-
> drivers/video/backlight/locomo_bl.c | 171 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 175 insertions(+), 4 deletions(-)
> create mode 100644 drivers/video/backlight/locomo_bl.c
>
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index 8d03924..03b77b33 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -218,12 +218,12 @@ config BACKLIGHT_LM3533
> levels.
>
> config BACKLIGHT_LOCOMO
> - tristate "Sharp LOCOMO LCD/Backlight Driver"
> - depends on SHARP_LOCOMO
> + tristate "Sharp LOCOMO Backlight Driver"
> + depends on MFD_LOCOMO
> default y
> help
> If you have a Sharp Zaurus SL-5500 (Collie) or SL-5600 (Poodle) say y to
> - enable the LCD/backlight driver.
> + enable the backlight driver.
>
> config BACKLIGHT_OMAP1
> tristate "OMAP1 PWL-based LCD Backlight"
> diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> index fcd50b73..2a61b7e 100644
> --- a/drivers/video/backlight/Makefile
> +++ b/drivers/video/backlight/Makefile
> @@ -39,7 +39,7 @@ obj-$(CONFIG_BACKLIGHT_IPAQ_MICRO) += ipaq_micro_bl.o
> obj-$(CONFIG_BACKLIGHT_LM3533) += lm3533_bl.o
> obj-$(CONFIG_BACKLIGHT_LM3630A) += lm3630a_bl.o
> obj-$(CONFIG_BACKLIGHT_LM3639) += lm3639_bl.o
> -obj-$(CONFIG_BACKLIGHT_LOCOMO) += locomolcd.o
> +obj-$(CONFIG_BACKLIGHT_LOCOMO) += locomo_bl.o
> obj-$(CONFIG_BACKLIGHT_LP855X) += lp855x_bl.o
> obj-$(CONFIG_BACKLIGHT_LP8788) += lp8788_bl.o
> obj-$(CONFIG_BACKLIGHT_LV5207LP) += lv5207lp.o
> diff --git a/drivers/video/backlight/locomo_bl.c b/drivers/video/backlight/locomo_bl.c
> new file mode 100644
> index 0000000..cec1b51
> --- /dev/null
> +++ b/drivers/video/backlight/locomo_bl.c
> @@ -0,0 +1,171 @@
> +/*
> + * Backlight control code for Sharp Zaurus SL-5500
> + *
> + * Copyright 2005 John Lenz <lenz@cs.wisc.edu>
> + * Maintainer: Pavel Machek <pavel@ucw.cz> (unless John wants to :-)
> + * GPL v2
> + *
> + * This driver assumes single CPU. That's okay, because collie is
> + * slightly old hardware, and no one is going to retrofit second CPU to
> + * old PDA.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/fb.h>
> +#include <linux/backlight.h>
> +#include <linux/err.h>
> +#include <linux/gpio.h>
> +#include <linux/delay.h>
> +#include <linux/mfd/locomo.h>
Please, re-order these headers alphabetically.
It enhances the readability.
> +
> +struct locomo_bl {
> + void __iomem *regs;
> + int current_intensity;
> + int gpio_fl_vr;
> +};
> +
> +static const struct {
> + u16 duty, bpwf;
> + bool vr;
> +} locomo_bl_pwm[] = {
> + { 0, 161, false },
> + { 117, 161, false },
> + { 163, 148, false },
> + { 194, 161, false },
> + { 194, 161, true },
> +};
> +
> +static int locomo_bl_set_intensity(struct backlight_device *bd)
> +{
> + int intensity = bd->props.brightness;
> + struct locomo_bl *bl = dev_get_drvdata(&bd->dev);
> +
> + if (bd->props.power != FB_BLANK_UNBLANK)
> + intensity = 0;
> + if (bd->props.fb_blank != FB_BLANK_UNBLANK)
> + intensity = 0;
> + if (bd->props.state & BL_CORE_SUSPENDED)
> + intensity = 0;
> +
> + gpio_set_value(bl->gpio_fl_vr, locomo_bl_pwm[intensity].vr);
> +
> + writew(locomo_bl_pwm[intensity].bpwf, bl->regs + LOCOMO_ALS);
> + udelay(100);
> + writew(locomo_bl_pwm[intensity].duty, bl->regs + LOCOMO_ALD);
> + udelay(100);
How about changing udelay() to usleep_range()?
> + writew(locomo_bl_pwm[intensity].bpwf | LOCOMO_ALC_EN,
> + bl->regs + LOCOMO_ALS);
> +
> + bl->current_intensity = intensity;
> + if (bd->props.state & BL_CORE_SUSPENDED)
> + writew(0x00, bl->regs + LOCOMO_ALS);
> +
> + return 0;
> +}
> +
> +static int locomo_bl_get_intensity(struct backlight_device *bd)
> +{
> + struct locomo_bl *bl = dev_get_drvdata(&bd->dev);
> +
> + return bl->current_intensity;
> +}
> +
> +static const struct backlight_ops locomo_bl_ops = {
> + .options = BL_CORE_SUSPENDRESUME,
> + .get_brightness = locomo_bl_get_intensity,
> + .update_status = locomo_bl_set_intensity,
> +};
> +
> +static int locomo_bl_probe(struct platform_device *dev)
> +{
> + struct backlight_properties props;
> + struct resource *res;
> + struct locomo_bl_platform_data *pdata;
> + struct locomo_bl *bl;
> + struct backlight_device *locomo_bl_device;
> + int rc;
> +
> + bl = devm_kmalloc(&dev->dev, sizeof(struct locomo_bl), GFP_KERNEL);
> + if (!bl)
> + return -ENOMEM;
> +
> + res = platform_get_resource(dev, IORESOURCE_MEM, 0);
> + if (!res)
> + return -EINVAL;
There is no need to check this, because devm_ioremap_resource()
will check this.
> + bl->regs = devm_ioremap_resource(&dev->dev, res);
> + if (!bl->regs)
> + return -EINVAL;
The correct usage is as follows.
if (IS_ERR(base))
return PTR_ERR(base);
Please change it as below.
res = platform_get_resource(dev, IORESOURCE_MEM, 0);
bl->regs = devm_ioremap_resource(&dev->dev, res);
if (IS_ERR(bl->regs))
return PTR_ERR(bl->regs);
> +
> + pdata = dev_get_platdata(&dev->dev);
> + if (!pdata)
> + return -EINVAL;
> +
> + bl->gpio_fl_vr = pdata->gpio_fl_vr;
> + rc = devm_gpio_request_one(&dev->dev, bl->gpio_fl_vr,
> + GPIOF_OUT_INIT_LOW, "FL VR");
> + if (rc)
> + return rc;
> +
> + writew(0, bl->regs + LOCOMO_ALS);
> + writew(0, bl->regs + LOCOMO_ALD);
> +
> + memset(&props, 0, sizeof(struct backlight_properties));
> + props.type = BACKLIGHT_RAW;
> + props.max_brightness = ARRAY_SIZE(locomo_bl_pwm) - 1;
> + props.brightness = props.max_brightness / 2;
> + locomo_bl_device = backlight_device_register("locomo-bl",
Please use devm_backlight_device_register().
> + &dev->dev, bl,
> + &locomo_bl_ops, &props);
> +
> + if (IS_ERR(locomo_bl_device))
> + return PTR_ERR(locomo_bl_device);
> +
> + platform_set_drvdata(dev, locomo_bl_device);
> +
> + /* Set up frontlight so that screen is readable */
> + backlight_update_status(locomo_bl_device);
> +
> + return 0;
> +}
> +
> +static int locomo_bl_remove(struct platform_device *dev)
> +{
> + struct backlight_device *locomo_bl_device = platform_get_drvdata(dev);
> +
> + locomo_bl_device->props.brightness = 0;
> + locomo_bl_device->props.power = 0;
> + locomo_bl_set_intensity(locomo_bl_device);
> +
> + backlight_device_unregister(locomo_bl_device);
If devm_backlight_device_register() is used in probe(),
there is no need to call backlight_device_unregister() in remove().
> +
> + return 0;
> +}
> +
> +static void locomo_bl_shutdown(struct platform_device *dev)
> +{
> + struct backlight_device *locomo_bl_device = platform_get_drvdata(dev);
> +
> + locomo_bl_device->props.brightness = 0;
> + locomo_bl_device->props.power = 0;
> + locomo_bl_set_intensity(locomo_bl_device);
> +}
> +
> +static struct platform_driver locomo_bl_driver = {
> + .driver = {
> + .name = "locomo-backlight",
> + .owner = THIS_MODULE,
> + },
> + .probe = locomo_bl_probe,
> + .remove = locomo_bl_remove,
> + /* Turn off bl on power off/reboot */
> + .shutdown = locomo_bl_shutdown,
> +};
> +
> +module_platform_driver(locomo_bl_driver);
> +
> +MODULE_AUTHOR("John Lenz <lenz@cs.wisc.edu>, Pavel Machek <pavel@ucw.cz>");
This might make checkpatch warning.
Please split these authors to lines as below.
MODULE_AUTHOR("John Lenz <lenz@cs.wisc.edu>");
MODULE_AUTHOR("Pavel Machek <pavel@ucw.cz>");
> +MODULE_DESCRIPTION("Collie Backlight driver");
What does mean 'Collie'? 'Locomo' looks better.
> +MODULE_LICENSE("GPL");
How about using "GPL v2"?
Thank you.
Best regards,
Jingoo Han
> +MODULE_ALIAS("platform:locomo-backlight");
> --
> 2.1.1
^ permalink raw reply
* Re: [PATCH 00/15] new locomo driver
From: Russell King - ARM Linux @ 2014-10-28 0:13 UTC (permalink / raw)
To: Dmitry Eremin-Solenikov, Greg KH
Cc: linux-arm-kernel, linux-gpio, linux-input, linux-leds, linux-spi,
linux-fbdev, alsa-devel, Andrea Adami, Daniel Mack,
Haojian Zhuang, Robert Jarzmik, Linus Walleij, Alexandre Courbot,
Dmitry Torokhov, Bryan Wu, Richard Purdie, Samuel Ortiz,
Lee Jones, Mark Brown, Jingoo Han, Liam Girdwood
In-Reply-To: <1414454528-24240-1-git-send-email-dbaryshkov@gmail.com>
On Tue, Oct 28, 2014 at 03:01:53AM +0300, Dmitry Eremin-Solenikov wrote:
> Sharp Zaurus SL-5500 and SL-5600 use special companion Gate Array. Current
> drivers present in Linux kernel has some problems:
>
> * It uses custom bus instead of platform bus/mfd core.
I believe Greg wouldn't see that as a positive point.
Don't think that the platform bus _should_ always be used. It shouldn't
(Greg has said he'd like to see the platform bus to be totally killed off.)
Instead, custom buses properly suited to the class of device in question
is much preferred, especially if it aids in...
> * Device drivers are not well layered/separated.
... better layering or separation of drivers.
So, thinking that converting from a custom bus to a platform bus
definitely is /not/ a positive step forward.
(Why mfd was ever allowed to re-use the platform bus stuff is a separate
question not relevent to these patches.)
--
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* Re: [PATCH 04/15] input: convert LoCoMo keyboard driver to use new locomo core
From: Dmitry Torokhov @ 2014-10-28 0:09 UTC (permalink / raw)
To: Dmitry Eremin-Solenikov
Cc: linux-arm-kernel, linux-gpio, linux-input, linux-leds, linux-spi,
linux-fbdev, alsa-devel, Andrea Adami, Russell King, Daniel Mack,
Haojian Zhuang, Robert Jarzmik, Linus Walleij, Alexandre Courbot,
Bryan Wu, Richard Purdie, Samuel Ortiz, Lee Jones, Mark Brown,
Jingoo Han, Liam Girdwood
In-Reply-To: <1414454528-24240-5-git-send-email-dbaryshkov@gmail.com>
On Tue, Oct 28, 2014 at 03:01:57AM +0300, Dmitry Eremin-Solenikov wrote:
> As LoCoMo is switching to new device model, adapt keyboard driver to
> support new locomo core driver.
>
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> ---
> drivers/input/keyboard/Kconfig | 1 -
> drivers/input/keyboard/locomokbd.c | 165 ++++++++++++++++++++-----------------
> 2 files changed, 91 insertions(+), 75 deletions(-)
>
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index a3958c6..b660516 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -337,7 +337,6 @@ config KEYBOARD_LM8333
>
> config KEYBOARD_LOCOMO
> tristate "LoCoMo Keyboard Support"
> - depends on SHARP_LOCOMO
> help
> Say Y here if you are running Linux on a Sharp Zaurus Collie or Poodle based PDA
>
> diff --git a/drivers/input/keyboard/locomokbd.c b/drivers/input/keyboard/locomokbd.c
> index c94d610..c2fabf3 100644
> --- a/drivers/input/keyboard/locomokbd.c
> +++ b/drivers/input/keyboard/locomokbd.c
> @@ -28,16 +28,10 @@
> #include <linux/init.h>
> #include <linux/input.h>
> #include <linux/delay.h>
> -#include <linux/device.h>
> +#include <linux/platform_device.h>
> #include <linux/interrupt.h>
> -#include <linux/ioport.h>
> -
> -#include <asm/hardware/locomo.h>
> -#include <asm/irq.h>
> -
> -MODULE_AUTHOR("John Lenz <lenz@cs.wisc.edu>");
> -MODULE_DESCRIPTION("LoCoMo keyboard driver");
> -MODULE_LICENSE("GPL");
> +#include <linux/io.h>
> +#include <linux/mfd/locomo.h>
>
> #define LOCOMOKBD_NUMKEYS 128
>
> @@ -75,7 +69,8 @@ struct locomokbd {
> struct input_dev *input;
> char phys[32];
>
> - unsigned long base;
> + void __iomem *base;
> + int irq;
> spinlock_t lock;
>
> struct timer_list timer;
> @@ -84,37 +79,37 @@ struct locomokbd {
> };
>
> /* helper functions for reading the keyboard matrix */
> -static inline void locomokbd_charge_all(unsigned long membase)
> +static inline void locomokbd_charge_all(void __iomem *membase)
> {
> - locomo_writel(0x00FF, membase + LOCOMO_KSC);
> + writew(0x00FF, membase + LOCOMO_KSC);
> }
>
> -static inline void locomokbd_activate_all(unsigned long membase)
> +static inline void locomokbd_activate_all(void __iomem *membase)
> {
> unsigned long r;
>
> - locomo_writel(0, membase + LOCOMO_KSC);
> - r = locomo_readl(membase + LOCOMO_KIC);
> + writew(0, membase + LOCOMO_KSC);
> + r = readw(membase + LOCOMO_KIC);
> r &= 0xFEFF;
> - locomo_writel(r, membase + LOCOMO_KIC);
> + writew(r, membase + LOCOMO_KIC);
> }
>
> -static inline void locomokbd_activate_col(unsigned long membase, int col)
> +static inline void locomokbd_activate_col(void __iomem *membase, int col)
> {
> unsigned short nset;
> unsigned short nbset;
>
> nset = 0xFF & ~(1 << col);
> nbset = (nset << 8) + nset;
> - locomo_writel(nbset, membase + LOCOMO_KSC);
> + writew(nbset, membase + LOCOMO_KSC);
> }
>
> -static inline void locomokbd_reset_col(unsigned long membase, int col)
> +static inline void locomokbd_reset_col(void __iomem *membase, int col)
> {
> unsigned short nbset;
>
> nbset = ((0xFF & ~(1 << col)) << 8) + 0xFF;
> - locomo_writel(nbset, membase + LOCOMO_KSC);
> + writew(nbset, membase + LOCOMO_KSC);
> }
>
> /*
> @@ -129,7 +124,7 @@ static void locomokbd_scankeyboard(struct locomokbd *locomokbd)
> unsigned int row, col, rowd;
> unsigned long flags;
> unsigned int num_pressed;
> - unsigned long membase = locomokbd->base;
> + void __iomem *membase = locomokbd->base;
>
> spin_lock_irqsave(&locomokbd->lock, flags);
>
> @@ -141,7 +136,7 @@ static void locomokbd_scankeyboard(struct locomokbd *locomokbd)
> locomokbd_activate_col(membase, col);
> udelay(KB_DELAY);
>
> - rowd = ~locomo_readl(membase + LOCOMO_KIB);
> + rowd = ~readw(membase + LOCOMO_KIB);
> for (row = 0; row < KB_ROWS; row++) {
> unsigned int scancode, pressed, key;
>
> @@ -194,11 +189,11 @@ static irqreturn_t locomokbd_interrupt(int irq, void *dev_id)
> struct locomokbd *locomokbd = dev_id;
> u16 r;
>
> - r = locomo_readl(locomokbd->base + LOCOMO_KIC);
> + r = readw(locomokbd->base + LOCOMO_KIC);
> if ((r & 0x0001) = 0)
> return IRQ_HANDLED;
>
> - locomo_writel(r & ~0x0100, locomokbd->base + LOCOMO_KIC); /* Ack */
> + writew(r & ~0x0100, locomokbd->base + LOCOMO_KIC); /* Ack */
>
> /** wait chattering delay **/
> udelay(100);
> @@ -222,8 +217,8 @@ static int locomokbd_open(struct input_dev *dev)
> struct locomokbd *locomokbd = input_get_drvdata(dev);
> u16 r;
>
> - r = locomo_readl(locomokbd->base + LOCOMO_KIC) | 0x0010;
> - locomo_writel(r, locomokbd->base + LOCOMO_KIC);
> + r = readw(locomokbd->base + LOCOMO_KIC) | 0x0010;
> + writew(r, locomokbd->base + LOCOMO_KIC);
> return 0;
> }
>
> @@ -232,35 +227,39 @@ static void locomokbd_close(struct input_dev *dev)
> struct locomokbd *locomokbd = input_get_drvdata(dev);
> u16 r;
>
> - r = locomo_readl(locomokbd->base + LOCOMO_KIC) & ~0x0010;
> - locomo_writel(r, locomokbd->base + LOCOMO_KIC);
> + r = readw(locomokbd->base + LOCOMO_KIC) & ~0x0010;
> + writew(r, locomokbd->base + LOCOMO_KIC);
> }
>
> -static int locomokbd_probe(struct locomo_dev *dev)
> +static int locomokbd_probe(struct platform_device *dev)
> {
> struct locomokbd *locomokbd;
> struct input_dev *input_dev;
> int i, err;
> + struct resource *res;
> +
> + locomokbd = devm_kzalloc(&dev->dev, sizeof(struct locomokbd),
> + GFP_KERNEL);
> + if (!locomokbd)
> + return -ENOMEM;
>
> - locomokbd = kzalloc(sizeof(struct locomokbd), GFP_KERNEL);
> input_dev = input_allocate_device();
> - if (!locomokbd || !input_dev) {
> - err = -ENOMEM;
> - goto err_free_mem;
> - }
> + if (!input_dev)
> + return -ENOMEM;
>
> - /* try and claim memory region */
> - if (!request_mem_region((unsigned long) dev->mapbase,
> - dev->length,
> - LOCOMO_DRIVER_NAME(dev))) {
> - err = -EBUSY;
> - printk(KERN_ERR "locomokbd: Can't acquire access to io memory for keyboard\n");
> - goto err_free_mem;
> - }
> + res = platform_get_resource(dev, IORESOURCE_MEM, 0);
> + if (!res)
> + return -ENODEV;
>
> - locomo_set_drvdata(dev, locomokbd);
> + locomokbd->irq = platform_get_irq(dev, 0);
> + if (locomokbd->irq < 0)
> + return -ENXIO;
>
> - locomokbd->base = (unsigned long) dev->mapbase;
> + platform_set_drvdata(dev, locomokbd);
> +
> + locomokbd->base = devm_ioremap_resource(&dev->dev, res);
> + if (IS_ERR(locomokbd->base))
> + return PTR_ERR(locomokbd->base);
You are leaking memory (input device) here. Since you can't convert all
resources to devm* I'd rather you leave them all explicitly managed
instead of having a mix.
Thanks.
--
Dmitry
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox