date:20130916

Re: [PATCH 4/6] Staging/iio/adc/touchscreen/MXS: add i.MX23 support to the LRADC touchscreen driver

2013-09-16 Thread Jürgen Beisert

Hi Jonathan,

On Sunday 15 September 2013 12:50:09 Jonathan Cameron wrote:
 On 09/11/13 09:18, Juergen Beisert wrote:
  Distinguish i.MX23 and i.MX28 at runtime and do the same for both SoC at
  least for the 4 wire touchscreen.
 
  Note: support for the remaining LRADC channels is not tested on an
  i.MX23 yet.

 An ominous comment. Are you likely to test them soon?

Sorry, currently no i.MX28 hardware available. Someone out here with an i.MX28 
based system with a touchscreen to test? 

 Minor code layout comment inline but otherwise I'm just looking for an
 ack from Marek.

  [...]
   static u32 mxs_lradc_drive_pressure(struct mxs_lradc *lradc)
   {
  +   if (lradc-soc == IMX23_LRADC)
  +   return LRADC_CTRL0_MX23_YP | LRADC_CTRL0_MX23_XM;
  return LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_XNNSW;

 Whilst it obviously doesn't actually matter, having an else
 in there would make the code more consistent so personally
 I would prefer it to be there.

I can change it. Anyone here with objections against such a change?

Regards
Juergen

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | http://www.pengutronix.de/  |
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 3/6] Staging/iio/adc/touchscreen/MXS: simplify register access

2013-09-16 Thread Jürgen Beisert

Hi Jonathan,

On Sunday 15 September 2013 12:35:29 Jonathan Cameron wrote:
 [...]
  +static void mxs_lradc_reg_set(struct mxs_lradc *lradc, u32 val, u32 reg)
  +{
  +   writel(val, lradc-base + reg + STMP_OFFSET_REG_SET);
  +}
  +
  +static void mxs_lradc_reg_clear(struct mxs_lradc *lradc, u32 val, u32 reg)
  +{ 
  +   writel(val, lradc-base + reg + STMP_OFFSET_REG_CLR);
  +}
  +

 mxs_lradc_reg_write might be clearer?

I would prefer the shorter name. Due to long macro names and long function 
names the lines in the code below needs more and more line breaks which is 
IMHO less helpfull to read the code.

Regards,
Juergen

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | http://www.pengutronix.de/  |
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/1] Drivers: input: serio: New driver to support Hyper-V synthetic keyboard

2013-09-16 Thread Dan Carpenter

The main thing is that could you improve the error handling in
hv_kbd_on_channel_callback() explained inline.

On Sun, Sep 15, 2013 at 10:28:54PM -0700, K. Y. Srinivasan wrote:
 Add a new driver to support synthetic keyboard. On the next generation
 Hyper-V guest firmware, many legacy devices will not be emulated and this
 driver will be required.
 
 I would like to thank Vojtech Pavlik vojt...@suse.cz for helping me with the
 details of the AT keyboard driver. 
 
 Signed-off-by: K. Y. Srinivasan k...@microsoft.com
 ---
  drivers/input/serio/Kconfig   |7 +
  drivers/input/serio/Makefile  |1 +
  drivers/input/serio/hyperv-keyboard.c |  379 
 +
  3 files changed, 387 insertions(+), 0 deletions(-)
  create mode 100644 drivers/input/serio/hyperv-keyboard.c
 
 diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
 index 1e691a3..f3996e7 100644
 --- a/drivers/input/serio/Kconfig
 +++ b/drivers/input/serio/Kconfig
 @@ -267,4 +267,11 @@ config SERIO_OLPC_APSP
 To compile this driver as a module, choose M here: the module will
 be called olpc_apsp.
  
 +config HYPERV_KEYBOARD
 + tristate Microsoft Synthetic Keyboard driver
 + depends on HYPERV
 + default HYPERV
 + help
 +   Select this option to enable the Hyper-V Keyboard driver.
 +
  endif
 diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
 index 12298b1..815d874 100644
 --- a/drivers/input/serio/Makefile
 +++ b/drivers/input/serio/Makefile
 @@ -28,3 +28,4 @@ obj-$(CONFIG_SERIO_ALTERA_PS2)  += altera_ps2.o
  obj-$(CONFIG_SERIO_ARC_PS2)  += arc_ps2.o
  obj-$(CONFIG_SERIO_APBPS2)   += apbps2.o
  obj-$(CONFIG_SERIO_OLPC_APSP)+= olpc_apsp.o
 +obj-$(CONFIG_HYPERV_KEYBOARD)+= hyperv-keyboard.o
 diff --git a/drivers/input/serio/hyperv-keyboard.c 
 b/drivers/input/serio/hyperv-keyboard.c
 new file mode 100644
 index 000..0d4625f
 --- /dev/null
 +++ b/drivers/input/serio/hyperv-keyboard.c
 @@ -0,0 +1,379 @@
 +/*
 + *  Copyright (c) 2013, Microsoft Corporation.
 + *
 + *  This program is free software; you can redistribute it and/or modify it
 + *  under the terms and conditions of the GNU General Public License,
 + *  version 2, as published by the Free Software Foundation.
 + *
 + *  This program is distributed in the hope 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/init.h
 +#include linux/module.h
 +#include linux/device.h
 +#include linux/completion.h
 +#include linux/hyperv.h
 +#include linux/serio.h
 +#include linux/slab.h
 +
 +

Extra blank line.

 +/*
 + * Current version 1.0
 + *
 + */
 +#define SYNTH_KBD_VERSION_MAJOR 1
 +#define SYNTH_KBD_VERSION_MINOR  0
 +#define SYNTH_KBD_VERSION(SYNTH_KBD_VERSION_MINOR | \
 +  (SYNTH_KBD_VERSION_MAJOR  16))
 +
 +
 +/*
 + * Message types in the synthetic input protocol
 + */
 +enum synth_kbd_msg_type {
 + SYNTH_KBD_PROTOCOL_REQUEST = 1,
 + SYNTH_KBD_PROTOCOL_RESPONSE = 2,
 + SYNTH_KBD_EVENT = 3,
 + SYNTH_KBD_LED_INDICATORS = 4,
 +};
 +
 +/*
 + * Basic message structures.
 + */
 +struct synth_kbd_msg_hdr {
 + enum synth_kbd_msg_type type;
 +};

Enum type is wrong here because sizeof(enum) is up to the compiler to
decide.

 +
 +struct synth_kbd_msg {
 + struct synth_kbd_msg_hdr header;
 + char data[1]; /* Enclosed message */
 +};

You could use a zero size array instead.

 +
 +union synth_kbd_version {
 + struct {
 + u16 minor_version;
 + u16 major_version;
 + };
 + u32 version;
 +};
 +
 +/*
 + * Protocol messages
 + */
 +struct synth_kbd_protocol_request {
 + struct synth_kbd_msg_hdr header;
 + union synth_kbd_version version_requested;
 +};
 +
 +struct synth_kbd_protocol_response {
 + struct synth_kbd_msg_hdr header;
 + u32 accepted:1;
 + u32 reserved:31;
 +};
 +
 +struct synth_kbd_keystroke {
 + struct synth_kbd_msg_hdr header;
 + u16 make_code;
 + u16 reserved0;
 + u32 is_unicode:1;
 + u32 is_break:1;
 + u32 is_e0:1;
 + u32 is_e1:1;
 + u32 reserved:28;
 +};
 +
 +

Extra blank line.

 +#define HK_MAXIMUM_MESSAGE_SIZE 256
 +
 +#define KBD_VSC_SEND_RING_BUFFER_SIZE(10 * PAGE_SIZE)
 +#define KBD_VSC_RECV_RING_BUFFER_SIZE(10 * PAGE_SIZE)
 +
 +#define XTKBD_EMUL0 0xe0
 +#define XTKBD_EMUL1 0xe1
 +#define XTKBD_RELEASE   0x80
 +
 +

Extra blank.

 +/*
 + * Represents a keyboard device
 + */
 +struct hv_kbd_dev {
 + unsigned char keycode[256];
 + struct hv_device*device;
 + struct synth_kbd_protocol_request protocol_req;
 + struct synth_kbd_protocol_response protocol_resp;
 + /* Synchronize the request/response if needed */
 + struct completion

Re: [PATCH 5/6] Staging/iio/adc/touchscreen/MXS: add interrupt driven touch detection

2013-09-16 Thread Jürgen Beisert

Hi Jonathan,

On Sunday 15 September 2013 18:10:18 Jonathan Cameron wrote:
On 09/15/13 11:56, Jonathan Cameron wrote:
On 09/11/13 09:18, Juergen Beisert wrote:
For battery driven systems it is a very bad idea to collect the
touchscreen data within a kernel busy loop.

This change uses the features of the hardware to delay and accumulate
samples in hardware to avoid a high interrupt and CPU load.

Note: this is only tested on an i.MX23 SoC yet.

Signed-off-by: Juergen Beisert j...@pengutronix.de
CC: linux-arm-ker...@lists.infradead.org
CC: de...@driverdev.osuosl.org
CC: Marek Vasut ma...@denx.de
CC: Fabio Estevam fabio.este...@freescale.com
CC: Jonathan Cameron ji...@cam.ac.uk

While this driver is placed in IIO within staging at the moment, these
changes are definitely input related. Hence I have cc'd Dmitry and the
input list.

I am personaly a little uncomfortable that we have such a complex bit of
input code sat within an IIO driver but such is life.

The logic in here looks reasonable to me. I am far from a specialist in how
these touch screens are normally handled though.

One thing to note is that you really want to get a proposed device tree
spec out asap as that can take longer to review than the driver. If you
are proposing to do that as a future patch, then take into account that
you'll need to ensure these are the defaults if it is not specified in the
device tree for ever more (which is more painful than hammering out he
device tree stuff now!)
...

Will do.

Regards,
Juergen

--
Pengutronix e.K. | Juergen Beisert |
Linux Solutions for Science and Industry | http://www.pengutronix.de/ |
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 5/6] Staging/iio/adc/touchscreen/MXS: add interrupt driven touch detection

2013-09-16 Thread Jürgen Beisert

Hi Jonathan,

On Monday 16 September 2013 10:10:22 Jürgen Beisert wrote:
 [...]
  While this driver is placed in IIO within staging at the moment, these
  changes are definitely input related.  Hence I have cc'd Dmitry and the
  input list.
 
  I am personaly a little uncomfortable that we have such a complex bit of
  input code sat within an IIO driver but such is life.

 Maybe an MFD for this ADC unit would be a better way to go? Currently I
 have a different problem with this driver, because the ADC unit monitors
 the battery as well. And the charging driver from the power subsystem needs
 these values to charge the battery in a correct manner.

I would suggest:

diff --git a/drivers/staging/iio/TODO b/drivers/staging/iio/TODO
index 04c2326..c22a0ed 100644
--- a/drivers/staging/iio/TODO
+++ b/drivers/staging/iio/TODO
@@ -13,6 +13,17 @@ Would be nice
 3) Expand device set. Lots of other maxim adc's have very
similar interfaces.
 
+MXS LRADC driver:
+This is a classic MFD device as it combines the following subdevices
+ - touchscreen controller (input subsystem related device)
+ - general purpose ADC channels
+ - battery voltage monitor (power subsystem related device)
+ - die temperature monitor (thermal management)
+
+At least the battery voltage and die temperature feature is required in-kernel
+by a driver of the SoC's battery charging unit to avoid any damage to the
+silicon and the battery.
+
 TSL2561
 Would be nice
 1) Open question of userspace vs kernel space balance when

Regards,
Juergen

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | http://www.pengutronix.de/  |
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4] mfd: rtsx: Modify rts5249_optimize_phy

2013-09-16 Thread Lee Jones

On Fri, 13 Sep 2013, wei_w...@realsil.com.cn wrote:

 From: Wei WANG wei_w...@realsil.com.cn
 
 In some platforms, specially Thinkpad series, rts5249 won't be
 initialized properly. So we need adjust some phy parameters to
 improve the compatibility issue.
 
 It is a little different between simulation and real chip. We have
 no idea about which configuration is better before tape-out. We set
 default settings according to simulation, but need to tune these
 parameters after getting the real chip.
 
 I can't explain every change in detail here. The below information is
 just a rough description:
 
 PHY_REG_REV: Disable internal clkreq_tx, enable rx_pwst
 PHY_BPCR: No change, just turn the magic number to macro definitions
 PHY_PCR: Change OOBS sensitivity, from 60mV to 90mV
 PHY_RCR2: Control charge-pump current automatically
 PHY_FLD4: Use TX cmu reference clock
 PHY_RDR: Change RXDSEL from 30nF to 1.9nF
 PHY_RCR1: Change the duration between adp_st and asserting cp_en from
 0.32 us to 0.64us
 PHY_FLD3: Adjust internal timers
 PHY_TUNE: Fine tune the regulator12 output voltage
 
 Signed-off-by: Wei WANG wei_w...@realsil.com.cn
 ---
  drivers/mfd/rts5249.c|   48 --
  include/linux/mfd/rtsx_pci.h |   53 
 ++
  2 files changed, 99 insertions(+), 2 deletions(-)

Much better.

Applied, thanks.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

51 matches

Mail list logo