Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH] HID: input: Fix TransducerSerialNumber implementation
From: Jason Gerecke @ 2014-10-28 18:31 UTC (permalink / raw)
  To: Benjamin Tissoires, Jason Gerecke, linux-input@vger.kernel.org,
	jkosina@suse.cz, pinglinux@gmail.com
  Cc: Jason Gerecke
In-Reply-To: <1411495768-5956-1-git-send-email-killertofu@gmail.com>

On Tue, Sep 23, 2014 at 11:09 AM, Jason Gerecke <killertofu@gmail.com> wrote:
> The commit which introduced TransducerSerialNumber (368c966) is missing
> two crucial implementation details. Firstly, the commit does not set the
> type/code/bit/max fields as expected later down the code which can cause
> the driver to crash when a tablet with this usage is connected. Secondly,
> the call to 'set_bit' causes MSC_PULSELED to be sent instead of the
> expected MSC_SERIAL. This commit addreses both issues.
>
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> ---
>  drivers/hid/hid-input.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 2619f7f..cb1b3fa 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -689,7 +689,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
>                         break;
>
>                 case 0x5b: /* TransducerSerialNumber */
> -                       set_bit(MSC_SERIAL, input->mscbit);
> +                       usage->type = EV_MSC;
> +                       usage->code = MSC_SERIAL;
> +                       bit = input->mscbit;
> +                       max = MSC_MAX;
>                         break;
>
>                 default:  goto unknown;
> --
> 2.1.0
>

This patch still seems to be in limbo. Anyone (Ping? Benjamin?) care
to provide an ACK or review?

Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one  /
(That is to say, eight) to the two,     /
But you can’t take seven from three,    /
So you look at the sixty-fours....
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2] Input: driver for the Goodix touchpanel
From: Dmitry Torokhov @ 2014-10-28 18:05 UTC (permalink / raw)
  To: Bastien Nocera; +Cc: Henrik Rydberg, linux-input, Benjamin Tissoires
In-Reply-To: <1414518921.2406.11.camel@hadess.net>

On Tue, Oct 28, 2014 at 06:55:21PM +0100, Bastien Nocera wrote:
> On Tue, 2014-10-07 at 13:58 -0700, Dmitry Torokhov wrote:
> > On Wed, Sep 24, 2014 at 04:43:58PM +0200, Bastien Nocera wrote:
> > > +static irqreturn_t goodix_ts_irq_handler(int irq, void *dev_id)
> > > +{
> > > +	struct goodix_ts_data *ts = dev_id;
> > > +	u8  end_cmd[1] = {0};
> > > +
> > > +	goodix_process_events(ts);
> > > +
> > > +	if (goodix_i2c_write(ts->client,
> > > +				GOODIX_READ_COOR_ADDR, end_cmd, 1) < 0)
> > > +		dev_err(&ts->client->dev, "I2C write end_cmd error");
> > 
> > I am not happy that we need to allocate/deallocate memory for each
> > interrupt. We only write one command to the driver, we could simply use
> > i2c_master_send() with a constant buffer.
> 
> Sure. But I've split up the patch you sent us, and committed the
> different bits separately in:
> https://github.com/hadess/gt9xx/commits/master
> 
> And this one commit about removing goodix_i2c_write():
> https://github.com/hadess/gt9xx/commit/146b4cc2eed5c67bcf1cb91e845bf9f97da4be1e
> 
> Breaks the driver.

Ah, I see. In end_cmd I encoded the address as little endian, whereas it
needs to be beg endian. Just swap "GOODIX_READ_COOR_ADDR & 0xff" and
"GOODIX_READ_COOR_ADDR >> 8" around and I thin kit will work.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v2] Input: driver for the Goodix touchpanel
From: Bastien Nocera @ 2014-10-28 17:55 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Henrik Rydberg, linux-input, Benjamin Tissoires
In-Reply-To: <20141007205803.GJ16469@dtor-ws>

On Tue, 2014-10-07 at 13:58 -0700, Dmitry Torokhov wrote:
> Hi Bastien,
> 
> On Wed, Sep 24, 2014 at 04:43:58PM +0200, Bastien Nocera wrote:
<snip>
> > +	if (touch_num > 1) {
> > +		ret = goodix_i2c_read(ts->client, GOODIX_READ_COOR_ADDR + 10,
> > +				   &data[10], 8 * (touch_num - 1));
> > +		if (ret < 0)
> > +			return ret;
> > +	}
> 
> I am a bit confused about this function. It looks like contact packet
> size is 8 bytes, and they preceded by a byte with total number of
> contacts reported, so why instead of 9 bytes we are reading 10?

I have no idea. We didn't change the original code here:
https://github.com/hadess/gt9xx/commit/82b141220e8bce00060e0de697735d0a70af2678#diff-5d71019f9b92cc9d6e2e31ed2e6520b6R363

<snip>
> > +/**
> > + * goodix_process_events - Process incoming events
> > + *
> > + * @ts: our goodix_ts_data pointer
> > + *
> > + * Called when the IRQ is triggered. Read the current device state, and push
> > + * the input events to the user space.
> > + */
> > +static void goodix_process_events(struct goodix_ts_data *ts)
> > +{
> > +	u8  point_data[1 + 8 * GOODIX_MAX_TOUCH + 1];
> 
> Here again, why do we need extra byte?

Same answer:
https://github.com/hadess/gt9xx/commit/82b141220e8bce00060e0de697735d0a70af2678#diff-5d71019f9b92cc9d6e2e31ed2e6520b6R315

<snip>
> > +/**
> > + * goodix_ts_irq_handler - The IRQ handler
> > + *
> > + * @irq: interrupt number.
> > + * @dev_id: private data pointer.
> > + */
> > +static irqreturn_t goodix_ts_irq_handler(int irq, void *dev_id)
> > +{
> > +	struct goodix_ts_data *ts = dev_id;
> > +	u8  end_cmd[1] = {0};
> > +
> > +	goodix_process_events(ts);
> > +
> > +	if (goodix_i2c_write(ts->client,
> > +				GOODIX_READ_COOR_ADDR, end_cmd, 1) < 0)
> > +		dev_err(&ts->client->dev, "I2C write end_cmd error");
> 
> I am not happy that we need to allocate/deallocate memory for each
> interrupt. We only write one command to the driver, we could simply use
> i2c_master_send() with a constant buffer.

Sure. But I've split up the patch you sent us, and committed the
different bits separately in:
https://github.com/hadess/gt9xx/commits/master

And this one commit about removing goodix_i2c_write():
https://github.com/hadess/gt9xx/commit/146b4cc2eed5c67bcf1cb91e845bf9f97da4be1e

Breaks the driver.

> BTW, you need terminate kernel messages with \n.

All of them? If so, we have a few more that are still missing in the
latest version of the driver, which I can take care of.

> Also, below is a patch with a few assorted changes that I'd like you to
> try if you have time.

As mentioned, all the changes seem fine apart from the one removing
goodix_i2c_write().

I'll test to see if we can reduce the size of the point_data structure,
and see what's breaking the goodix_i2c_write() patch.

Thanks for the patch!

Cheers


^ permalink raw reply

* Re: [PATCH] Input: nomadik-ske-keypad: Switch to using managed resources
From: Pramod Gurav @ 2014-10-28 17:51 UTC (permalink / raw)
  To: Pramod Gurav; +Cc: linux-kernel@vger.kernel.org, Dmitry Torokhov, linux-input
In-Reply-To: <1412765428-6517-1-git-send-email-pramod.gurav@smartplayin.com>

Hi Dmitry,
Is this change okey?

On Wed, Oct 8, 2014 at 4:20 PM, Pramod Gurav
<pramod.gurav@smartplayin.com> wrote:
> This change switches to using devm_* APIs to allocate  resources.
> This helps to simplify failure path in probe function as well as
> remove function.
>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: linux-input@vger.kernel.org
> Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
> ---
>  drivers/input/keyboard/nomadik-ske-keypad.c |   63 +++++++++------------------
>  1 file changed, 20 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/input/keyboard/nomadik-ske-keypad.c b/drivers/input/keyboard/nomadik-ske-keypad.c
> index 63332e2..95ac317 100644
> --- a/drivers/input/keyboard/nomadik-ske-keypad.c
> +++ b/drivers/input/keyboard/nomadik-ske-keypad.c
> @@ -247,12 +247,11 @@ static int __init ske_keypad_probe(struct platform_device *pdev)
>                 return -EINVAL;
>         }
>
> -       keypad = kzalloc(sizeof(struct ske_keypad), GFP_KERNEL);
> +       keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad), GFP_KERNEL);
>         input = input_allocate_device();
>         if (!keypad || !input) {
>                 dev_err(&pdev->dev, "failed to allocate keypad memory\n");
> -               error = -ENOMEM;
> -               goto err_free_mem;
> +               return -ENOMEM;
>         }
>
>         keypad->irq = irq;
> @@ -260,31 +259,29 @@ static int __init ske_keypad_probe(struct platform_device *pdev)
>         keypad->input = input;
>         spin_lock_init(&keypad->ske_keypad_lock);
>
> -       if (!request_mem_region(res->start, resource_size(res), pdev->name)) {
> +       if (!devm_request_mem_region(&pdev->dev, res->start,
> +                                    resource_size(res), pdev->name)) {
>                 dev_err(&pdev->dev, "failed to request I/O memory\n");
> -               error = -EBUSY;
> -               goto err_free_mem;
> +               return -EBUSY;
>         }
>
> -       keypad->reg_base = ioremap(res->start, resource_size(res));
> +       keypad->reg_base = devm_ioremap(&pdev->dev, res->start,
> +                                       resource_size(res));
>         if (!keypad->reg_base) {
>                 dev_err(&pdev->dev, "failed to remap I/O memory\n");
> -               error = -ENXIO;
> -               goto err_free_mem_region;
> +               return -ENXIO;
>         }
>
> -       keypad->pclk = clk_get(&pdev->dev, "apb_pclk");
> +       keypad->pclk = devm_clk_get(&pdev->dev, "apb_pclk");
>         if (IS_ERR(keypad->pclk)) {
>                 dev_err(&pdev->dev, "failed to get pclk\n");
> -               error = PTR_ERR(keypad->pclk);
> -               goto err_iounmap;
> +               return PTR_ERR(keypad->pclk);
>         }
>
> -       keypad->clk = clk_get(&pdev->dev, NULL);
> +       keypad->clk = devm_clk_get(&pdev->dev, NULL);
>         if (IS_ERR(keypad->clk)) {
>                 dev_err(&pdev->dev, "failed to get clk\n");
> -               error = PTR_ERR(keypad->clk);
> -               goto err_pclk;
> +               return PTR_ERR(keypad->clk);
>         }
>
>         input->id.bustype = BUS_HOST;
> @@ -296,7 +293,7 @@ static int __init ske_keypad_probe(struct platform_device *pdev)
>                                            keypad->keymap, input);
>         if (error) {
>                 dev_err(&pdev->dev, "Failed to build keymap\n");
> -               goto err_clk;
> +               return error;
>         }
>
>         input_set_capability(input, EV_MSC, MSC_SCAN);
> @@ -306,7 +303,7 @@ static int __init ske_keypad_probe(struct platform_device *pdev)
>         error = clk_prepare_enable(keypad->pclk);
>         if (error) {
>                 dev_err(&pdev->dev, "Failed to prepare/enable pclk\n");
> -               goto err_clk;
> +               return error;
>         }
>
>         error = clk_prepare_enable(keypad->clk);
> @@ -326,8 +323,9 @@ static int __init ske_keypad_probe(struct platform_device *pdev)
>                 goto err_clk_disable;
>         }
>
> -       error = request_threaded_irq(keypad->irq, NULL, ske_keypad_irq,
> -                                    IRQF_ONESHOT, "ske-keypad", keypad);
> +       error = devm_request_threaded_irq(&pdev->dev, keypad->irq, NULL,
> +                                         ske_keypad_irq, IRQF_ONESHOT,
> +                                         "ske-keypad", keypad);
>         if (error) {
>                 dev_err(&pdev->dev, "allocate irq %d failed\n", keypad->irq);
>                 goto err_clk_disable;
> @@ -337,7 +335,7 @@ static int __init ske_keypad_probe(struct platform_device *pdev)
>         if (error) {
>                 dev_err(&pdev->dev,
>                                 "unable to register input device: %d\n", error);
> -               goto err_free_irq;
> +               goto err_clk_disable;
>         }
>
>         if (plat->wakeup_enable)
> @@ -347,45 +345,24 @@ static int __init ske_keypad_probe(struct platform_device *pdev)
>
>         return 0;
>
> -err_free_irq:
> -       free_irq(keypad->irq, keypad);
>  err_clk_disable:
>         clk_disable_unprepare(keypad->clk);
>  err_pclk_disable:
>         clk_disable_unprepare(keypad->pclk);
> -err_clk:
> -       clk_put(keypad->clk);
> -err_pclk:
> -       clk_put(keypad->pclk);
> -err_iounmap:
> -       iounmap(keypad->reg_base);
> -err_free_mem_region:
> -       release_mem_region(res->start, resource_size(res));
> -err_free_mem:
> -       input_free_device(input);
> -       kfree(keypad);
> +
>         return error;
>  }
>
>  static int ske_keypad_remove(struct platform_device *pdev)
>  {
>         struct ske_keypad *keypad = platform_get_drvdata(pdev);
> -       struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -
> -       free_irq(keypad->irq, keypad);
> -
> -       input_unregister_device(keypad->input);
>
>         clk_disable_unprepare(keypad->clk);
> -       clk_put(keypad->clk);
> +       clk_disable_unprepare(keypad->pclk);
>
>         if (keypad->board->exit)
>                 keypad->board->exit();
>
> -       iounmap(keypad->reg_base);
> -       release_mem_region(res->start, resource_size(res));
> -       kfree(keypad);
> -
>         return 0;
>  }
>
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
Thanks and Regards
Pramod

^ 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-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>
> with at least the subject line fixed.

Subject line fixed.

-- 
With best wishes
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH] Input: max77693-haptic - fix potential overflow
From: Dmitry Torokhov @ 2014-10-28 16:43 UTC (permalink / raw)
  To: linux-input; +Cc: Chanwoo Choi, Jaewon Kim, linux-kernel

Expression haptic->pwm_dev->period * haptic->magnitude is of type 'unsigned
int' and may overflow. We need to convert one of the operands to u64 before
multiplying, instead of casting result (potentially overflown) to u64.

Reported by Coverity: CID 1248753

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/misc/max77693-haptic.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/input/misc/max77693-haptic.c b/drivers/input/misc/max77693-haptic.c
index 7b1fde9..ef6a9d6 100644
--- a/drivers/input/misc/max77693-haptic.c
+++ b/drivers/input/misc/max77693-haptic.c
@@ -194,7 +194,7 @@ static int max77693_haptic_play_effect(struct input_dev *dev, void *data,
 				       struct ff_effect *effect)
 {
 	struct max77693_haptic *haptic = input_get_drvdata(dev);
-	uint64_t period_mag_multi;
+	u64 period_mag_multi;
 
 	haptic->magnitude = effect->u.rumble.strong_magnitude;
 	if (!haptic->magnitude)
@@ -205,8 +205,7 @@ static int max77693_haptic_play_effect(struct input_dev *dev, void *data,
 	 * The formula to convert magnitude to pwm_duty as follows:
 	 * - pwm_duty = (magnitude * pwm_period) / MAX_MAGNITUDE(0xFFFF)
 	 */
-	period_mag_multi = (int64_t)(haptic->pwm_dev->period *
-						haptic->magnitude);
+	period_mag_multi = (u64)haptic->pwm_dev->period * haptic->magnitude;
 	haptic->pwm_duty = (unsigned int)(period_mag_multi >>
 						MAX_MAGNITUDE_SHIFT);
 
-- 
2.1.0.rc2.206.gedb03e5


-- 
Dmitry

^ permalink raw reply related

* 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: 736 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-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

with at least the subject line fixed.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ 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] Input: spear-keyboard - Add CONFIG_PM_SLEEP to suspend/resume functions
From: Jingoo Han @ 2014-10-28  1:43 UTC (permalink / raw)
  To: 'Dmitry Torokhov'
  Cc: linux-input, 'Viresh Kumar', 'Jingoo Han'
In-Reply-To: <71CA30BA-E2BE-45D3-9FA8-6E1E9B1284A9@gmail.com>



> -----Original Message-----
> From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> Sent: Tuesday, October 28, 2014 10:09 AM
> To: Jingoo Han
> Cc: linux-input@vger.kernel.org; 'Viresh Kumar'; 'Jingoo Han'
> Subject: Re: [PATCH] Input: spear-keyboard - Add CONFIG_PM_SLEEP to suspend/resume functions
> 
> On October 27, 2014 5:53:56 PM PDT, Jingoo Han <jg1.han@samsung.com> wrote:
> >On Tuesday, October 28, 2014 9:11 AM, Dmitry Torokhov wrote:
> >> On Tue, Oct 28, 2014 at 09:02:44AM +0900, Jingoo Han wrote:
> >> > On Tuesday, October 28, 2014 8:38 AM, Dmitry Torokhov wrote:
> >> > > On Mon, Oct 27, 2014 at 09:32:58PM +0900, Jingoo Han wrote:
> >> > > > Add CONFIG_PM_SLEEP to suspend/resume functions
> >> > > >
> >> > > > Add CONFIG_PM_SLEEP to suspend/resume functions instead of
> >> > > > CONFIG_PM to fix the following build warning when
> >CONFIG_PM_SLEEP
> >> > > > is not selected and CONFIG_PM is selected. This is because
> >sleep
> >> > > > PM callbacks defined by SIMPLE_DEV_PM_OPS are only used when
> >the
> >> > > > CONFIG_PM_SLEEP is enabled.
> >> > >
> >> > > Recently I've become a fan of __maybe_unused markings as they
> >insulate
> >> > > us from various CONFIG changes in unrelated subsystems, I'll
> >transform
> >> > > this patch to use them instead.
> >> >
> >> > OK, I see. I have no objection.
> >> > Then, how about changing other usages of CONFIG_PM_SLEEP/CONFIG_PM
> >> > to __maybe_unused annotation? Personally, I prefer to increase
> >build
> >> > coverage than using #ifdef guards. Someone, however, argued that
> >#ifdef
> >> > guards should be used in this case because the size of binary can
> >be
> >> > reduced. How about your opinion?
> >>
> >> The optimizer is supposed to drop functions marked as
> >'__maybe_unused'
> >> if they are indeed unused so size of the binary should not change.
> >
> >Sorry for annoying you.
> >I built 'spear-keyboard' and got the binaries as below.
> >
> >spear-keyboard.o 89500 bytes, <-- CONFIG_PM_SLEEP=n
> >spear-keyboard.o 92352 bytes, <-- CONFIG_PM_SLEEP=y
> >spear-keyboard.o 92352 bytes, <-- Marked as __maybe_unused
> >
> >As presented above, when CONFIG_PM_SLEEP is used instead of
> >__maybe_unused, the size of binary is reduced. So, someone
> >complained me to use #ifdef guards. But, I agree with your
> >opinion. Personally, I DON'T want to use #ifdef guards,
> >because I prefer to increase build coverage.
> 
> What about the final kernel image size/kernel module size?

I used 'spear13xx_defconfig' with linux-next(20141027).
Then, I changed '#ifdef CONFIG_PM' into '#ifdef CONFIG_PM_SLEEP'
as below.

-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
 static int spear_kbd_suspend(struct device *dev)
 {


Then, I got the following kernel images.

1. CONFIG_PM_SLEEP=y
    zImage 2891128 bytes
    spear-keyboard.o 88528 bytes

2. CONFIG_PM_SLEEP=n
    zImage 2842656 bytes
    spear-keyboard.o 85980 bytes

3. Marked as '__maybe_unused' for 'spear_kbd_suspend()/spear_kbd_resume()'
    instead of using '#ifdef CONFIG_PM_SLEEP',
    and CONFIG_PM_SLEEP=y
    zImage 2891160 bytes
    spear-keyboard.o 88528 bytes

4. Marked as '__maybe_unused' for 'spear_kbd_suspend()/spear_kbd_resume()'
    instead of using '#ifdef CONFIG_PM_SLEEP',
    and CONFIG_PM_SLEEP=n
    zImage 2842712 bytes
    spear-keyboard.o 85980 bytes

Oh, sorry! Your comment is right.
As you mentioned, the total sizes of the kernel and spear-keyboard.o
are reduced, when '__maybe_unused' annotation is used.
Thank you!

Best regards,
Jingoo Han

> 
> 
> Thanks.
> 
> --
> Dmitry


^ permalink raw reply

* Re: [PATCH] Input: spear-keyboard - Add CONFIG_PM_SLEEP to suspend/resume functions
From: Dmitry Torokhov @ 2014-10-28  1:08 UTC (permalink / raw)
  Cc: linux-input, 'Viresh Kumar', 'Jingoo Han'
In-Reply-To: <000701cff249$a700dde0$f50299a0$%han@samsung.com>

On October 27, 2014 5:53:56 PM PDT, Jingoo Han <jg1.han@samsung.com> wrote:
>On Tuesday, October 28, 2014 9:11 AM, Dmitry Torokhov wrote:
>> On Tue, Oct 28, 2014 at 09:02:44AM +0900, Jingoo Han wrote:
>> > On Tuesday, October 28, 2014 8:38 AM, Dmitry Torokhov wrote:
>> > > On Mon, Oct 27, 2014 at 09:32:58PM +0900, Jingoo Han wrote:
>> > > > Add CONFIG_PM_SLEEP to suspend/resume functions
>> > > >
>> > > > Add CONFIG_PM_SLEEP to suspend/resume functions instead of
>> > > > CONFIG_PM to fix the following build warning when
>CONFIG_PM_SLEEP
>> > > > is not selected and CONFIG_PM is selected. This is because
>sleep
>> > > > PM callbacks defined by SIMPLE_DEV_PM_OPS are only used when
>the
>> > > > CONFIG_PM_SLEEP is enabled.
>> > >
>> > > Recently I've become a fan of __maybe_unused markings as they
>insulate
>> > > us from various CONFIG changes in unrelated subsystems, I'll
>transform
>> > > this patch to use them instead.
>> >
>> > OK, I see. I have no objection.
>> > Then, how about changing other usages of CONFIG_PM_SLEEP/CONFIG_PM
>> > to __maybe_unused annotation? Personally, I prefer to increase
>build
>> > coverage than using #ifdef guards. Someone, however, argued that
>#ifdef
>> > guards should be used in this case because the size of binary can
>be
>> > reduced. How about your opinion?
>> 
>> The optimizer is supposed to drop functions marked as
>'__maybe_unused'
>> if they are indeed unused so size of the binary should not change.
>
>Sorry for annoying you.
>I built 'spear-keyboard' and got the binaries as below.
>
>spear-keyboard.o 89500 bytes, <-- CONFIG_PM_SLEEP=n
>spear-keyboard.o 92352 bytes, <-- CONFIG_PM_SLEEP=y
>spear-keyboard.o 92352 bytes, <-- Marked as __maybe_unused
>
>As presented above, when CONFIG_PM_SLEEP is used instead of
>__maybe_unused, the size of binary is reduced. So, someone
>complained me to use #ifdef guards. But, I agree with your
>opinion. Personally, I DON'T want to use #ifdef guards,
>because I prefer to increase build coverage.

What about the final kernel image size/kernel module size?


Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] Input: spear-keyboard - Add CONFIG_PM_SLEEP to suspend/resume functions
From: Jingoo Han @ 2014-10-28  0:53 UTC (permalink / raw)
  To: 'Dmitry Torokhov'
  Cc: linux-input, 'Viresh Kumar', 'Jingoo Han'
In-Reply-To: <20141028001100.GF7594@dtor-ws>

On Tuesday, October 28, 2014 9:11 AM, Dmitry Torokhov wrote:
> On Tue, Oct 28, 2014 at 09:02:44AM +0900, Jingoo Han wrote:
> > On Tuesday, October 28, 2014 8:38 AM, Dmitry Torokhov wrote:
> > > On Mon, Oct 27, 2014 at 09:32:58PM +0900, Jingoo Han wrote:
> > > > Add CONFIG_PM_SLEEP to suspend/resume functions
> > > >
> > > > Add CONFIG_PM_SLEEP to suspend/resume functions instead of
> > > > CONFIG_PM to fix the following build warning when CONFIG_PM_SLEEP
> > > > is not selected and CONFIG_PM is selected. This is because sleep
> > > > PM callbacks defined by SIMPLE_DEV_PM_OPS are only used when the
> > > > CONFIG_PM_SLEEP is enabled.
> > >
> > > Recently I've become a fan of __maybe_unused markings as they insulate
> > > us from various CONFIG changes in unrelated subsystems, I'll transform
> > > this patch to use them instead.
> >
> > OK, I see. I have no objection.
> > Then, how about changing other usages of CONFIG_PM_SLEEP/CONFIG_PM
> > to __maybe_unused annotation? Personally, I prefer to increase build
> > coverage than using #ifdef guards. Someone, however, argued that #ifdef
> > guards should be used in this case because the size of binary can be
> > reduced. How about your opinion?
> 
> The optimizer is supposed to drop functions marked as '__maybe_unused'
> if they are indeed unused so size of the binary should not change.

Sorry for annoying you.
I built 'spear-keyboard' and got the binaries as below.

spear-keyboard.o 89500 bytes, <-- CONFIG_PM_SLEEP=n
spear-keyboard.o 92352 bytes, <-- CONFIG_PM_SLEEP=y
spear-keyboard.o 92352 bytes, <-- Marked as __maybe_unused

As presented above, when CONFIG_PM_SLEEP is used instead of
__maybe_unused, the size of binary is reduced. So, someone
complained me to use #ifdef guards. But, I agree with your
opinion. Personally, I DON'T want to use #ifdef guards,
because I prefer to increase build coverage.
Thank you.

> 
> Thanks.
> 
> --
> Dmitry


^ permalink raw reply

* Re: [PATCH] Input: spear-keyboard - Add CONFIG_PM_SLEEP to suspend/resume functions
From: Dmitry Torokhov @ 2014-10-28  0:35 UTC (permalink / raw)
  To: Jingoo Han; +Cc: linux-input, 'Viresh Kumar'
In-Reply-To: <000601cff246$e192a3b0$a4b7eb10$%han@samsung.com>

On Tuesday, October 28, 2014 09:34:06 AM Jingoo Han wrote:
> On Tuesday, October 28, 2014 9:11 AM, Dmitry Torokhov wrote:
> > On Tue, Oct 28, 2014 at 09:02:44AM +0900, Jingoo Han wrote:
> > > On Tuesday, October 28, 2014 8:38 AM, Dmitry Torokhov wrote:
> > > > On Mon, Oct 27, 2014 at 09:32:58PM +0900, Jingoo Han wrote:
> > > > > Add CONFIG_PM_SLEEP to suspend/resume functions
> > > > > 
> > > > > Add CONFIG_PM_SLEEP to suspend/resume functions instead of
> > > > > CONFIG_PM to fix the following build warning when CONFIG_PM_SLEEP
> > > > > is not selected and CONFIG_PM is selected. This is because sleep
> > > > > PM callbacks defined by SIMPLE_DEV_PM_OPS are only used when the
> > > > > CONFIG_PM_SLEEP is enabled.
> > > > 
> > > > Recently I've become a fan of __maybe_unused markings as they insulate
> > > > us from various CONFIG changes in unrelated subsystems, I'll transform
> > > > this patch to use them instead.
> > > 
> > > OK, I see. I have no objection.
> > > Then, how about changing other usages of CONFIG_PM_SLEEP/CONFIG_PM
> > > to __maybe_unused annotation? Personally, I prefer to increase build
> > > coverage than using #ifdef guards. Someone, however, argued that #ifdef
> > > guards should be used in this case because the size of binary can be
> > > reduced. How about your opinion?
> > 
> > The optimizer is supposed to drop functions marked as '__maybe_unused'
> > if they are indeed unused so size of the binary should not change.
> 
> Oh, I really appreciate your reply. :-)
> Then, how about changing other usages of CONFIG_PM_SLEEP/CONFIG_PM
> to __maybe_unused annotation? If you don't have any objection, I will
> send one single patch for this.

That will work.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] Input: spear-keyboard - Add CONFIG_PM_SLEEP to suspend/resume functions
From: Jingoo Han @ 2014-10-28  0:34 UTC (permalink / raw)
  To: 'Dmitry Torokhov'
  Cc: linux-input, 'Viresh Kumar', 'Jingoo Han'
In-Reply-To: <20141028001100.GF7594@dtor-ws>

On Tuesday, October 28, 2014 9:11 AM, Dmitry Torokhov wrote:
> On Tue, Oct 28, 2014 at 09:02:44AM +0900, Jingoo Han wrote:
> > On Tuesday, October 28, 2014 8:38 AM, Dmitry Torokhov wrote:
> > > On Mon, Oct 27, 2014 at 09:32:58PM +0900, Jingoo Han wrote:
> > > > Add CONFIG_PM_SLEEP to suspend/resume functions
> > > >
> > > > Add CONFIG_PM_SLEEP to suspend/resume functions instead of
> > > > CONFIG_PM to fix the following build warning when CONFIG_PM_SLEEP
> > > > is not selected and CONFIG_PM is selected. This is because sleep
> > > > PM callbacks defined by SIMPLE_DEV_PM_OPS are only used when the
> > > > CONFIG_PM_SLEEP is enabled.
> > >
> > > Recently I've become a fan of __maybe_unused markings as they insulate
> > > us from various CONFIG changes in unrelated subsystems, I'll transform
> > > this patch to use them instead.
> >
> > OK, I see. I have no objection.
> > Then, how about changing other usages of CONFIG_PM_SLEEP/CONFIG_PM
> > to __maybe_unused annotation? Personally, I prefer to increase build
> > coverage than using #ifdef guards. Someone, however, argued that #ifdef
> > guards should be used in this case because the size of binary can be
> > reduced. How about your opinion?
> 
> The optimizer is supposed to drop functions marked as '__maybe_unused'
> if they are indeed unused so size of the binary should not change.

Oh, I really appreciate your reply. :-)
Then, how about changing other usages of CONFIG_PM_SLEEP/CONFIG_PM
to __maybe_unused annotation? If you don't have any objection, I will
send one single patch for this.

Best regards,
Jingoo Han

> 
> Thanks.
> 
> --
> Dmitry


^ 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-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>:
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ 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] Input: spear-keyboard - Add CONFIG_PM_SLEEP to suspend/resume functions
From: 'Dmitry Torokhov' @ 2014-10-28  0:11 UTC (permalink / raw)
  To: Jingoo Han; +Cc: linux-input, 'Viresh Kumar'
In-Reply-To: <000001cff242$80476680$80d63380$%han@samsung.com>

On Tue, Oct 28, 2014 at 09:02:44AM +0900, Jingoo Han wrote:
> On Tuesday, October 28, 2014 8:38 AM, Dmitry Torokhov wrote:
> > On Mon, Oct 27, 2014 at 09:32:58PM +0900, Jingoo Han wrote:
> > > Add CONFIG_PM_SLEEP to suspend/resume functions
> > >
> > > Add CONFIG_PM_SLEEP to suspend/resume functions instead of
> > > CONFIG_PM to fix the following build warning when CONFIG_PM_SLEEP
> > > is not selected and CONFIG_PM is selected. This is because sleep
> > > PM callbacks defined by SIMPLE_DEV_PM_OPS are only used when the
> > > CONFIG_PM_SLEEP is enabled.
> > 
> > Recently I've become a fan of __maybe_unused markings as they insulate
> > us from various CONFIG changes in unrelated subsystems, I'll transform
> > this patch to use them instead.
> 
> OK, I see. I have no objection.
> Then, how about changing other usages of CONFIG_PM_SLEEP/CONFIG_PM
> to __maybe_unused annotation? Personally, I prefer to increase build
> coverage than using #ifdef guards. Someone, however, argued that #ifdef
> guards should be used in this case because the size of binary can be
> reduced. How about your opinion?

The optimizer is supposed to drop functions marked as '__maybe_unused'
if they are indeed unused so size of the binary should not change.

Thanks.

-- 
Dmitry

^ 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

* [PATCH 07/15] video: backlight: drop old locomo bl/lcd driver
From: Dmitry Eremin-Solenikov @ 2014-10-28  0:02 UTC (permalink / raw)
  To: linux-arm-kernel, linux-gpio, linux-input, linux-leds, linux-spi,
	linux-fbdev, alsa-devel
  Cc: 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,
	Jingoo Han, Liam Girdwood
In-Reply-To: <1414454528-24240-1-git-send-email-dbaryshkov@gmail.com>

Old locomolcd driver is now completely obsolete by new locomo_bl and
locomo_lcd drivers, so let's drop it.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 drivers/video/backlight/locomolcd.c | 255 ------------------------------------
 1 file changed, 255 deletions(-)
 delete mode 100644 drivers/video/backlight/locomolcd.c

diff --git a/drivers/video/backlight/locomolcd.c b/drivers/video/backlight/locomolcd.c
deleted file mode 100644
index 6c3ec42..0000000
--- a/drivers/video/backlight/locomolcd.c
+++ /dev/null
@@ -1,255 +0,0 @@
-/*
- * 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/delay.h>
-#include <linux/device.h>
-#include <linux/interrupt.h>
-#include <linux/fb.h>
-#include <linux/backlight.h>
-
-#include <asm/hardware/locomo.h>
-#include <asm/irq.h>
-#include <asm/mach/sharpsl_param.h>
-#include <asm/mach-types.h>
-
-#include "../../../arch/arm/mach-sa1100/generic.h"
-
-static struct backlight_device *locomolcd_bl_device;
-static struct locomo_dev *locomolcd_dev;
-static unsigned long locomolcd_flags;
-#define LOCOMOLCD_SUSPENDED     0x01
-
-static void locomolcd_on(int comadj)
-{
-	locomo_gpio_set_dir(locomolcd_dev->dev.parent, LOCOMO_GPIO_LCD_VSHA_ON, 0);
-	locomo_gpio_write(locomolcd_dev->dev.parent, LOCOMO_GPIO_LCD_VSHA_ON, 1);
-	mdelay(2);
-
-	locomo_gpio_set_dir(locomolcd_dev->dev.parent, LOCOMO_GPIO_LCD_VSHD_ON, 0);
-	locomo_gpio_write(locomolcd_dev->dev.parent, LOCOMO_GPIO_LCD_VSHD_ON, 1);
-	mdelay(2);
-
-	locomo_m62332_senddata(locomolcd_dev, comadj, 0);
-	mdelay(5);
-
-	locomo_gpio_set_dir(locomolcd_dev->dev.parent, LOCOMO_GPIO_LCD_VEE_ON, 0);
-	locomo_gpio_write(locomolcd_dev->dev.parent, LOCOMO_GPIO_LCD_VEE_ON, 1);
-	mdelay(10);
-
-	/* TFTCRST | CPSOUT=0 | CPSEN */
-	locomo_writel(0x01, locomolcd_dev->mapbase + LOCOMO_TC);
-
-	/* Set CPSD */
-	locomo_writel(6, locomolcd_dev->mapbase + LOCOMO_CPSD);
-
-	/* TFTCRST | CPSOUT=0 | CPSEN */
-	locomo_writel((0x04 | 0x01), locomolcd_dev->mapbase + LOCOMO_TC);
-	mdelay(10);
-
-	locomo_gpio_set_dir(locomolcd_dev->dev.parent, LOCOMO_GPIO_LCD_MOD, 0);
-	locomo_gpio_write(locomolcd_dev->dev.parent, LOCOMO_GPIO_LCD_MOD, 1);
-}
-
-static void locomolcd_off(int comadj)
-{
-	/* TFTCRST=1 | CPSOUT=1 | CPSEN = 0 */
-	locomo_writel(0x06, locomolcd_dev->mapbase + LOCOMO_TC);
-	mdelay(1);
-
-	locomo_gpio_write(locomolcd_dev->dev.parent, LOCOMO_GPIO_LCD_VSHA_ON, 0);
-	mdelay(110);
-
-	locomo_gpio_write(locomolcd_dev->dev.parent, LOCOMO_GPIO_LCD_VEE_ON, 0);
-	mdelay(700);
-
-	/* TFTCRST=0 | CPSOUT=0 | CPSEN = 0 */
-	locomo_writel(0, locomolcd_dev->mapbase + LOCOMO_TC);
-	locomo_gpio_write(locomolcd_dev->dev.parent, LOCOMO_GPIO_LCD_MOD, 0);
-	locomo_gpio_write(locomolcd_dev->dev.parent, LOCOMO_GPIO_LCD_VSHD_ON, 0);
-}
-
-void locomolcd_power(int on)
-{
-	int comadj = sharpsl_param.comadj;
-	unsigned long flags;
-
-	local_irq_save(flags);
-
-	if (!locomolcd_dev) {
-		local_irq_restore(flags);
-		return;
-	}
-
-	/* read comadj */
-	if (comadj == -1 && machine_is_collie())
-		comadj = 128;
-	if (comadj == -1 && machine_is_poodle())
-		comadj = 118;
-
-	if (on)
-		locomolcd_on(comadj);
-	else
-		locomolcd_off(comadj);
-
-	local_irq_restore(flags);
-}
-EXPORT_SYMBOL(locomolcd_power);
-
-static int current_intensity;
-
-static int locomolcd_set_intensity(struct backlight_device *bd)
-{
-	int intensity = bd->props.brightness;
-
-	if (bd->props.power != FB_BLANK_UNBLANK)
-		intensity = 0;
-	if (bd->props.fb_blank != FB_BLANK_UNBLANK)
-		intensity = 0;
-	if (locomolcd_flags & LOCOMOLCD_SUSPENDED)
-		intensity = 0;
-
-	switch (intensity) {
-	/*
-	 * AC and non-AC are handled differently,
-	 * but produce same results in sharp code?
-	 */
-	case 0:
-		locomo_frontlight_set(locomolcd_dev, 0, 0, 161);
-		break;
-	case 1:
-		locomo_frontlight_set(locomolcd_dev, 117, 0, 161);
-		break;
-	case 2:
-		locomo_frontlight_set(locomolcd_dev, 163, 0, 148);
-		break;
-	case 3:
-		locomo_frontlight_set(locomolcd_dev, 194, 0, 161);
-		break;
-	case 4:
-		locomo_frontlight_set(locomolcd_dev, 194, 1, 161);
-		break;
-	default:
-		return -ENODEV;
-	}
-	current_intensity = intensity;
-	return 0;
-}
-
-static int locomolcd_get_intensity(struct backlight_device *bd)
-{
-	return current_intensity;
-}
-
-static const struct backlight_ops locomobl_data = {
-	.get_brightness = locomolcd_get_intensity,
-	.update_status  = locomolcd_set_intensity,
-};
-
-#ifdef CONFIG_PM_SLEEP
-static int locomolcd_suspend(struct device *dev)
-{
-	locomolcd_flags |= LOCOMOLCD_SUSPENDED;
-	locomolcd_set_intensity(locomolcd_bl_device);
-	return 0;
-}
-
-static int locomolcd_resume(struct device *dev)
-{
-	locomolcd_flags &= ~LOCOMOLCD_SUSPENDED;
-	locomolcd_set_intensity(locomolcd_bl_device);
-	return 0;
-}
-#endif
-
-static SIMPLE_DEV_PM_OPS(locomolcd_pm_ops, locomolcd_suspend, locomolcd_resume);
-
-static int locomolcd_probe(struct locomo_dev *ldev)
-{
-	struct backlight_properties props;
-	unsigned long flags;
-
-	local_irq_save(flags);
-	locomolcd_dev = ldev;
-
-	locomo_gpio_set_dir(ldev->dev.parent, LOCOMO_GPIO_FL_VR, 0);
-
-	/*
-	 * the poodle_lcd_power function is called for the first time
-	 * from fs_initcall, which is before locomo is activated.
-	 * We need to recall poodle_lcd_power here
-	 */
-	if (machine_is_poodle())
-		locomolcd_power(1);
-
-	local_irq_restore(flags);
-
-	memset(&props, 0, sizeof(struct backlight_properties));
-	props.type = BACKLIGHT_RAW;
-	props.max_brightness = 4;
-	locomolcd_bl_device = backlight_device_register("locomo-bl",
-							&ldev->dev, NULL,
-							&locomobl_data, &props);
-
-	if (IS_ERR(locomolcd_bl_device))
-		return PTR_ERR(locomolcd_bl_device);
-
-	/* Set up frontlight so that screen is readable */
-	locomolcd_bl_device->props.brightness = 2;
-	locomolcd_set_intensity(locomolcd_bl_device);
-
-	return 0;
-}
-
-static int locomolcd_remove(struct locomo_dev *dev)
-{
-	unsigned long flags;
-
-	locomolcd_bl_device->props.brightness = 0;
-	locomolcd_bl_device->props.power = 0;
-	locomolcd_set_intensity(locomolcd_bl_device);
-
-	backlight_device_unregister(locomolcd_bl_device);
-	local_irq_save(flags);
-	locomolcd_dev = NULL;
-	local_irq_restore(flags);
-	return 0;
-}
-
-static struct locomo_driver poodle_lcd_driver = {
-	.drv = {
-		.name	= "locomo-backlight",
-		.pm	= &locomolcd_pm_ops,
-	},
-	.devid	= LOCOMO_DEVID_BACKLIGHT,
-	.probe	= locomolcd_probe,
-	.remove	= locomolcd_remove,
-};
-
-static int __init locomolcd_init(void)
-{
-	return locomo_driver_register(&poodle_lcd_driver);
-}
-
-static void __exit locomolcd_exit(void)
-{
-	locomo_driver_unregister(&poodle_lcd_driver);
-}
-
-module_init(locomolcd_init);
-module_exit(locomolcd_exit);
-
-MODULE_AUTHOR("John Lenz <lenz@cs.wisc.edu>, Pavel Machek <pavel@ucw.cz>");
-MODULE_DESCRIPTION("Collie LCD driver");
-MODULE_LICENSE("GPL");
-- 
2.1.1


^ permalink raw reply related

* [PATCH 05/15] video: backlight: add new locomo backlight driver
From: Dmitry Eremin-Solenikov @ 2014-10-28  0:01 UTC (permalink / raw)
  To: linux-arm-kernel, linux-gpio, linux-input, linux-leds, linux-spi,
	linux-fbdev, alsa-devel
  Cc: 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,
	Jingoo Han, Liam Girdwood
In-Reply-To: <1414454528-24240-1-git-send-email-dbaryshkov@gmail.com>

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>
+
+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);
+	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;
+	bl->regs = devm_ioremap_resource(&dev->dev, res);
+	if (!bl->regs)
+		return -EINVAL;
+
+	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",
+						&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);
+
+	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>");
+MODULE_DESCRIPTION("Collie Backlight driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:locomo-backlight");
-- 
2.1.1


^ permalink raw reply related

* Re: [PATCH] Input: spear-keyboard - Add CONFIG_PM_SLEEP to suspend/resume functions
From: Jingoo Han @ 2014-10-28  0:02 UTC (permalink / raw)
  To: 'Dmitry Torokhov'
  Cc: linux-input, 'Viresh Kumar', 'Jingoo Han'
In-Reply-To: <20141027233744.GC7594@dtor-ws>

On Tuesday, October 28, 2014 8:38 AM, Dmitry Torokhov wrote:
> On Mon, Oct 27, 2014 at 09:32:58PM +0900, Jingoo Han wrote:
> > Add CONFIG_PM_SLEEP to suspend/resume functions
> >
> > Add CONFIG_PM_SLEEP to suspend/resume functions instead of
> > CONFIG_PM to fix the following build warning when CONFIG_PM_SLEEP
> > is not selected and CONFIG_PM is selected. This is because sleep
> > PM callbacks defined by SIMPLE_DEV_PM_OPS are only used when the
> > CONFIG_PM_SLEEP is enabled.
> 
> Recently I've become a fan of __maybe_unused markings as they insulate
> us from various CONFIG changes in unrelated subsystems, I'll transform
> this patch to use them instead.

OK, I see. I have no objection.
Then, how about changing other usages of CONFIG_PM_SLEEP/CONFIG_PM
to __maybe_unused annotation? Personally, I prefer to increase build
coverage than using #ifdef guards. Someone, however, argued that #ifdef
guards should be used in this case because the size of binary can be
reduced. How about your opinion?
Thank you.

Best regards,
Jingoo Han

> 
> Thanks.
> 
> >
> > drivers/input/keyboard/spear-keyboard.c:292:12: warning: 'spear_kbd_suspend' defined but not used [-
> Wunused-function]
> > drivers/input/keyboard/spear-keyboard.c:345:12: warning: 'spear_kbd_resume' defined but not used [-
> Wunused-function]
> >
> > Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> > ---
> >  drivers/input/keyboard/spear-keyboard.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/input/keyboard/spear-keyboard.c b/drivers/input/keyboard/spear-keyboard.c
> > index 258af10..fc93e79 100644
> > --- a/drivers/input/keyboard/spear-keyboard.c
> > +++ b/drivers/input/keyboard/spear-keyboard.c
> > @@ -288,7 +288,7 @@ static int spear_kbd_remove(struct platform_device *pdev)
> >  	return 0;
> >  }
> >
> > -#ifdef CONFIG_PM
> > +#ifdef CONFIG_PM_SLEEP
> >  static int spear_kbd_suspend(struct device *dev)
> >  {
> >  	struct platform_device *pdev = to_platform_device(dev);
> > --
> > 1.7.9.5
> >
> >
> 
> --
> Dmitry


^ permalink raw reply

* [PATCH 15/15] spi: add locomo SPI driver
From: Dmitry Eremin-Solenikov @ 2014-10-28  0:02 UTC (permalink / raw)
  To: 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
  Cc: 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,
	Jingoo Han, Liam Girdwood
In-Reply-To: <1414454528-24240-1-git-send-email-dbaryshkov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

LoCoMo chip has a built-in simple SPI controller. On Sharp SL-5500 PDDAs
it is connected to external MMC slot.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/spi/Kconfig        |   8 +
 drivers/spi/Makefile       |   1 +
 drivers/spi/spi-locomo.c   | 370 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/locomo.h |  30 ++++
 4 files changed, 409 insertions(+)
 create mode 100644 drivers/spi/spi-locomo.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 84e7c9e..1395780 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -242,6 +242,14 @@ config SPI_LM70_LLP
 	  which interfaces to an LM70 temperature sensor using
 	  a parallel port.
 
+config SPI_LOCOMO
+	tristate "Locomo SPI master"
+	depends on MFD_LOCOMO
+	select SPI_BITBANG
+	help
+	  This enables using the SPI controller as present in the LoCoMo
+	  chips. It is probably only usefull on the Sharp SL-5x00 PDA family.
+
 config SPI_MPC52xx
 	tristate "Freescale MPC52xx SPI (non-PSC) controller support"
 	depends on PPC_MPC52xx
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 78f24ca..4f96197 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -42,6 +42,7 @@ obj-$(CONFIG_SPI_FSL_SPI)		+= spi-fsl-spi.o
 obj-$(CONFIG_SPI_GPIO)			+= spi-gpio.o
 obj-$(CONFIG_SPI_IMX)			+= spi-imx.o
 obj-$(CONFIG_SPI_LM70_LLP)		+= spi-lm70llp.o
+obj-$(CONFIG_SPI_LOCOMO)		+= spi-locomo.o
 obj-$(CONFIG_SPI_MPC512x_PSC)		+= spi-mpc512x-psc.o
 obj-$(CONFIG_SPI_MPC52xx_PSC)		+= spi-mpc52xx-psc.o
 obj-$(CONFIG_SPI_MPC52xx)		+= spi-mpc52xx.o
diff --git a/drivers/spi/spi-locomo.c b/drivers/spi/spi-locomo.c
new file mode 100644
index 0000000..8e9e5c4
--- /dev/null
+++ b/drivers/spi/spi-locomo.c
@@ -0,0 +1,370 @@
+/*
+ * 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; version 2 of the License.
+ *
+ * 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/kernel.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/spi/spi.h>
+#include <linux/spi/spi_bitbang.h>
+#include <linux/mfd/locomo.h>
+#include <linux/delay.h>
+
+struct locomospi_dev {
+	struct spi_bitbang bitbang;
+	void __iomem *base;
+	int clock_base;
+	int clock_div;
+
+	u16 save_ct;
+	u16 save_md;
+};
+
+static void locomospi_reg_open(struct locomospi_dev *spidev)
+{
+	u16 r;
+
+	spidev->clock_div = DIV_64;
+	spidev->clock_base = CLOCK_18MHZ;
+	writew(LOCOMO_SPIMD_MSB1ST | LOCOMO_SPIMD_DOSTAT | LOCOMO_SPIMD_RCPOL |
+		  LOCOMO_SPIMD_TCPOL | (spidev->clock_base << 3) |
+		  spidev->clock_div,
+		  spidev->base + LOCOMO_SPIMD);
+
+	/* 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);
+	/* } */
+
+	writew(LOCOMO_SPICT_CS, spidev->base + LOCOMO_SPICT);
+
+	r = readw(spidev->base + LOCOMO_SPICT);
+	r |= (LOCOMO_SPICT_CEN | LOCOMO_SPICT_RXUEN | LOCOMO_SPICT_ALIGNEN);
+	writew(r, spidev->base + LOCOMO_SPICT);
+
+	udelay(200);
+
+	r = readw(spidev->base + LOCOMO_SPICT);
+	writew(r, spidev->base + LOCOMO_SPICT);
+
+	r = readw(spidev->base + LOCOMO_SPICT);
+	r &= ~LOCOMO_SPICT_CS;
+	writew(r, spidev->base + LOCOMO_SPICT);
+}
+
+static void locomospi_reg_release(struct locomospi_dev *spidev)
+{
+	u16 r;
+
+	r = readw(spidev->base + LOCOMO_SPICT);
+	r &= ~LOCOMO_SPICT_CEN;
+	writew(r, spidev->base + LOCOMO_SPICT);
+
+	r = readw(spidev->base + LOCOMO_SPIMD);
+	r &= ~LOCOMO_SPIMD_XEN;
+	writew(r, spidev->base + LOCOMO_SPIMD);
+
+	r = readw(spidev->base + LOCOMO_SPIMD);
+	r &= ~LOCOMO_SPIMD_XON;
+	writew(r, spidev->base + LOCOMO_SPIMD);
+
+	r = readw(spidev->base + LOCOMO_SPICT);
+	r |= LOCOMO_SPIMD_XEN; /* FIXME */
+	writew(r, spidev->base + LOCOMO_SPICT);
+}
+
+
+static void locomospi_chipselect(struct spi_device *spi, int is_active)
+{
+	struct locomospi_dev *spidev;
+	u16 r;
+
+	dev_dbg(&spi->dev, "SPI cs: %d\n", is_active);
+
+	spidev = spi_master_get_devdata(spi->master);
+
+	r = readw(spidev->base + LOCOMO_SPICT);
+	if (!!is_active ^ !!(spi->mode & SPI_CS_HIGH))
+		r &= ~LOCOMO_SPICT_CS;
+	else
+		r |= LOCOMO_SPICT_CS;
+	writew(r, spidev->base + LOCOMO_SPICT);
+}
+
+static u32 locomospi_txrx_word(struct spi_device *spi,
+		unsigned nsecs,
+		u32 word, u8 bits)
+{
+	struct locomospi_dev *spidev;
+	int wait;
+	int j;
+	u32 rx;
+
+	spidev = spi_master_get_devdata(spi->master);
+
+	if (spidev->clock_div == 4)
+		wait = 0x10000;
+	else
+		wait = 8;
+
+	for (j = 0; j < wait; j++) {
+		if (readw(spidev->base + LOCOMO_SPIST) & LOCOMO_SPI_RFW)
+			break;
+	}
+
+	writeb(word, spidev->base + LOCOMO_SPITD);
+	ndelay(nsecs);
+
+	for (j = 0; j < wait; j++) {
+		if (readw(spidev->base + LOCOMO_SPIST) & LOCOMO_SPI_RFR)
+			break;
+	}
+
+	rx = readb(spidev->base + LOCOMO_SPIRD);
+	ndelay(nsecs);
+
+	dev_dbg(&spi->dev, "SPI txrx: %02x/%02x\n", word, rx);
+
+	return rx;
+}
+
+static void locomo_spi_set_speed(struct locomospi_dev *spidev, u32 hz)
+{
+	u16 r;
+
+	if (hz >= 24576000) {
+		spidev->clock_base = CLOCK_25MHZ;
+		spidev->clock_div = DIV_1;
+	} else if (hz >= 22579200) {
+		spidev->clock_base = CLOCK_22MHZ;
+		spidev->clock_div = DIV_1;
+	} else if (hz >= 18432000) {
+		spidev->clock_base = CLOCK_18MHZ;
+		spidev->clock_div = DIV_1;
+	} else if (hz >= 12288000) {
+		spidev->clock_base = CLOCK_25MHZ;
+		spidev->clock_div = DIV_2;
+	} else if (hz >= 11289600) {
+		spidev->clock_base = CLOCK_22MHZ;
+		spidev->clock_div = DIV_2;
+	} else if (hz >= 9216000) {
+		spidev->clock_base = CLOCK_18MHZ;
+		spidev->clock_div = DIV_2;
+	} else if (hz >= 6144000) {
+		spidev->clock_base = CLOCK_25MHZ;
+		spidev->clock_div = DIV_4;
+	} else if (hz >= 5644800) {
+		spidev->clock_base = CLOCK_22MHZ;
+		spidev->clock_div = DIV_4;
+	} else if (hz >= 4608000) {
+		spidev->clock_base = CLOCK_18MHZ;
+		spidev->clock_div = DIV_4;
+	} else if (hz >= 3072000) {
+		spidev->clock_base = CLOCK_25MHZ;
+		spidev->clock_div = DIV_8;
+	} else if (hz >= 2822400) {
+		spidev->clock_base = CLOCK_22MHZ;
+		spidev->clock_div = DIV_8;
+	} else if (hz >= 2304000) {
+		spidev->clock_base = CLOCK_18MHZ;
+		spidev->clock_div = DIV_8;
+	} else if (hz >= 384000) {
+		spidev->clock_base = CLOCK_25MHZ;
+		spidev->clock_div = DIV_64;
+	} else if (hz >= 352800) {
+		spidev->clock_base = CLOCK_22MHZ;
+		spidev->clock_div = DIV_64;
+	} else {		/* set to 288 Khz */
+		spidev->clock_base = CLOCK_18MHZ;
+		spidev->clock_div = DIV_64;
+	}
+
+	r = readw(spidev->base + LOCOMO_SPIMD);
+	if ((r & LOCOMO_SPIMD_CLKSEL) == spidev->clock_div &&
+			(r & LOCOMO_SPIMD_XSEL) == (spidev->clock_div << 3))
+		return;
+
+	r &= ~(LOCOMO_SPIMD_XSEL | LOCOMO_SPIMD_CLKSEL | LOCOMO_SPIMD_XEN);
+	writew(r, spidev->base + LOCOMO_SPIMD);
+
+	r |= (spidev->clock_div | (spidev->clock_base << 3) | LOCOMO_SPIMD_XEN);
+	writew(r, spidev->base + LOCOMO_SPIMD);
+
+	udelay(300);
+}
+
+static int locomo_spi_setup_transfer(struct spi_device *spi,
+		struct spi_transfer *t)
+{
+	struct locomospi_dev *spidev;
+	u16 r;
+	u32 hz = 0;
+	int rc = spi_bitbang_setup_transfer(spi, t);
+
+	if (rc)
+		return rc;
+
+	if (t)
+		hz = t->speed_hz;
+	if (!hz)
+		hz = spi->max_speed_hz;
+
+	spidev = spi_master_get_devdata(spi->master);
+
+	r = readw(spidev->base + LOCOMO_SPIMD);
+	if (hz == 0) {
+		r &= ~LOCOMO_SPIMD_XON;
+		writew(r, spidev->base + LOCOMO_SPIMD);
+	} else {
+		r |= LOCOMO_SPIMD_XON;
+		writew(r, spidev->base + LOCOMO_SPIMD);
+		locomo_spi_set_speed(spidev, hz);
+	}
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int locomo_spi_suspend(struct device *dev)
+{
+	struct spi_master *master = dev_get_drvdata(dev);
+	struct locomospi_dev *spidev = spi_master_get_devdata(master);
+	int ret;
+
+	/* Stop the queue running */
+	ret = spi_master_suspend(master);
+	if (ret) {
+		dev_warn(dev, "cannot suspend master\n");
+		return ret;
+	}
+
+	spidev->save_ct = readw(spidev->base + LOCOMO_SPICT);
+	writew(0x40, spidev->base + LOCOMO_SPICT);
+
+	spidev->save_md = readw(spidev->base + LOCOMO_SPIMD);
+	writew(0x3c14, spidev->base + LOCOMO_SPIMD);
+
+	return 0;
+}
+
+static int locomo_spi_resume(struct device *dev)
+{
+	struct spi_master *master = dev_get_drvdata(dev);
+	struct locomospi_dev *spidev = spi_master_get_devdata(master);
+	int ret;
+
+	writew(spidev->save_ct, spidev->base + LOCOMO_SPICT);
+	writew(spidev->save_md, spidev->base + LOCOMO_SPIMD);
+
+	/* Start the queue running */
+	ret = spi_master_resume(master);
+	if (ret)
+		dev_err(dev, "problem starting queue (%d)\n", ret);
+
+	return ret;
+}
+
+static SIMPLE_DEV_PM_OPS(locomo_spi_pm_ops,
+		locomo_spi_suspend, locomo_spi_resume);
+
+#define LOCOMO_SPI_PM_OPS	(&locomo_spi_pm_ops)
+#else
+#define LOCOMO_SPI_PM_OPS	NULL
+#endif
+
+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");
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENODEV;
+
+	master = spi_alloc_master(&pdev->dev, sizeof(struct locomospi_dev));
+	if (!master)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, master);
+
+	master->bits_per_word_mask = SPI_BPW_RANGE_MASK(8, 8);
+	master->bus_num = 0;
+	master->num_chipselect = 1;
+
+	spidev = spi_master_get_devdata(master);
+	spidev->bitbang.master = spi_master_get(master);
+
+	spidev->bitbang.setup_transfer = locomo_spi_setup_transfer;
+	spidev->bitbang.chipselect = locomospi_chipselect;
+	spidev->bitbang.txrx_word[SPI_MODE_0] = locomospi_txrx_word;
+	spidev->bitbang.txrx_word[SPI_MODE_1] = locomospi_txrx_word;
+	spidev->bitbang.txrx_word[SPI_MODE_2] = locomospi_txrx_word;
+	spidev->bitbang.txrx_word[SPI_MODE_3] = locomospi_txrx_word;
+
+	spidev->bitbang.master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
+
+	spidev->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(spidev->base)) {
+		ret = PTR_ERR(spidev->base);
+		goto out_put;
+	}
+
+	locomospi_reg_open(spidev);
+
+	ret = spi_bitbang_start(&spidev->bitbang);
+	if (ret) {
+		dev_err(&pdev->dev, "bitbang start failed with %d\n", ret);
+		goto out_put;
+	}
+
+	return 0;
+
+out_put:
+	spi_master_put(master);
+	return ret;
+}
+
+static int locomo_spi_remove(struct platform_device *pdev)
+{
+	struct spi_master *master = platform_get_drvdata(pdev);
+	struct locomospi_dev *spidev = spi_master_get_devdata(master);
+
+	spi_bitbang_stop(&spidev->bitbang);
+	locomospi_reg_release(spidev);
+	spi_master_put(master);
+
+
+	return 0;
+}
+
+static struct platform_driver locomo_spi_driver = {
+	.probe = locomo_spi_probe,
+	.remove = locomo_spi_remove,
+	.driver = {
+		.name = "locomo-spi",
+		.owner = THIS_MODULE,
+		.pm	= LOCOMO_SPI_PM_OPS,
+	},
+};
+module_platform_driver(locomo_spi_driver);
+
+MODULE_AUTHOR("Thomas Kunze thommy-Pp6lxjB34Lw@public.gmane.org");
+MODULE_DESCRIPTION("LoCoMo SPI driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:locomo-spi");
diff --git a/include/linux/mfd/locomo.h b/include/linux/mfd/locomo.h
index 1f54300..f822d77 100644
--- a/include/linux/mfd/locomo.h
+++ b/include/linux/mfd/locomo.h
@@ -53,7 +53,37 @@
 /* SPI interface */
 #define LOCOMO_SPI	0x60
 #define LOCOMO_SPIMD	0x00		/* SPI mode setting */
+#define LOCOMO_SPIMD_LOOPBACK (1 << 15)	/* loopback tx to rx */
+#define LOCOMO_SPIMD_MSB1ST   (1 << 14)	/* send MSB first */
+#define LOCOMO_SPIMD_DOSTAT   (1 << 13)	/* transmit line is idle high */
+#define LOCOMO_SPIMD_TCPOL    (1 << 11)	/* transmit CPOL (maybe affects CPHA too) */
+#define LOCOMO_SPIMD_RCPOL    (1 << 10)	/* receive CPOL (maybe affects CPHA too) */
+#define	LOCOMO_SPIMD_TDINV    (1 << 9)	/* invert transmit line */
+#define LOCOMO_SPIMD_RDINV    (1 << 8)	/* invert receive line */
+#define LOCOMO_SPIMD_XON      (1 << 7)	/* enable spi controller clock */
+#define LOCOMO_SPIMD_XEN      (1 << 6)	/* clock bit write enable xon must be off, wait 300 us before xon->1 */
+#define LOCOMO_SPIMD_XSEL     0x0018	/* clock select */
+#define CLOCK_18MHZ	    0		/* 18,432 MHz clock */
+#define CLOCK_22MHZ	    1		/* 22,5792 MHz clock */
+#define CLOCK_25MHZ	    2		/* 24,576 MHz clock */
+#define LOCOMO_SPIMD_CLKSEL   0x7
+#define DIV_1		    0		/* don't divide clock   */
+#define DIV_2		    1		/* divide clock by two	*/
+#define DIV_4		    2		/* divide clock by four */
+#define DIV_8		    3		/* divide clock by eight*/
+#define DIV_64		    4		/* divide clock by 64 */
+
 #define LOCOMO_SPICT	0x04		/* SPI mode control */
+#define LOCOMO_SPICT_CRC16_7_B	(1 << 15)	/* 0: crc16 1: crc7 */
+#define LOCOMO_SPICT_CRCRX_TX_B	(1 << 14)
+#define LOCOMO_SPICT_CRCRESET_B	(1 << 13)
+#define LOCOMO_SPICT_CEN	(1 << 7)	/* ?? enable */
+#define LOCOMO_SPICT_CS		(1 << 6)	/* chip select */
+#define LOCOMO_SPICT_UNIT16	(1 << 5)	/* 0: 8 bit units, 1: 16 bit unit */
+#define LOCOMO_SPICT_ALIGNEN	(1 << 2)	/* align transfer enable */
+#define LOCOMO_SPICT_RXWEN	(1 << 1)	/* continous receive */
+#define LOCOMO_SPICT_RXUEN	(1 << 0)	/* aligned receive */
+
 #define LOCOMO_SPIST	0x08		/* SPI status */
 #define	LOCOMO_SPI_TEND	(1 << 3)	/* Transfer end bit */
 #define	LOCOMO_SPI_REND	(1 << 2)	/* Receive end bit */
-- 
2.1.1

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

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox