* [linux-sunxi] [PATCH v4 0/2] Initial Allwinner V3s CSI Support
From: Ondřej Jirman @ 2017-12-22 13:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1513935138-35223-1-git-send-email-yong.deng@magewell.com>
Hello,
Yong Deng p??e v P? 22. 12. 2017 v 17:32 +0800:
> This patchset add initial support for Allwinner V3s CSI.
>
> Allwinner V3s SoC have two CSI module. CSI0 is used for MIPI interface
> and CSI1 is used for parallel interface. This is not documented in
> datasheet but by testing and guess.
>
> This patchset implement a v4l2 framework driver and add a binding
> documentation for it.
>
> Currently, the driver only support the parallel interface. And has been
> tested with a BT1120 signal which generating from FPGA. The following
> fetures are not support with this patchset:
> - ISP
> - MIPI-CSI2
> - Master clock for camera sensor
> - Power regulator for the front end IC
>
> Thanks for Ond?ej Jirman's help.
>
> Changes in v4:
> * Deal with the CSI 'INNER QUEUE'.
> CSI will lookup the next dma buffer for next frame before the
> the current frame done IRQ triggered. This is not documented
> but reported by Ond?ej Jirman.
> The BSP code has workaround for this too. It skip to mark the
> first buffer as frame done for VB2 and pass the second buffer
> to CSI in the first frame done ISR call. Then in second frame
> done ISR call, it mark the first buffer as frame done for VB2
> and pass the third buffer to CSI. And so on. The bad thing is
> that the first buffer will be written twice and the first frame
> is dropped even the queued buffer is sufficient.
> So, I make some improvement here. Pass the next buffer to CSI
> just follow starting the CSI. In this case, the first frame
> will be stored in first buffer, second frame in second buffer.
> This mothed is used to avoid dropping the first frame, it
> would also drop frame when lacking of queued buffer.
> * Fix: using a wrong mbus_code when getting the supported formats
> * Change all fourcc to pixformat
> * Change some function names
>
> Changes in v3:
> * Get rid of struct sun6i_csi_ops
> * Move sun6i-csi to new directory drivers/media/platform/sunxi
> * Merge sun6i_csi.c and sun6i_csi_v3s.c into sun6i_csi.c
> * Use generic fwnode endpoints parser
> * Only support a single subdev to make things simple
> * Many complaintion fix
>
> Changes in v2:
> * Change sunxi-csi to sun6i-csi
> * Rebase to media_tree master branch
>
> Following is the 'v4l2-compliance -s -f' output, I have test this
> with both interlaced and progressive signal:
>
> # ./v4l2-compliance -s -f
> v4l2-compliance SHA : 6049ea8bd64f9d78ef87ef0c2b3dc9b5de1ca4a1
>
> Driver Info:
> Driver name : sun6i-video
> Card type : sun6i-csi
> Bus info : platform:csi
> Driver version: 4.15.0
> Capabilities : 0x84200001
> Video Capture
> Streaming
> Extended Pix Format
> Device Capabilities
> Device Caps : 0x04200001
> Video Capture
> Streaming
> Extended Pix Format
>
> Compliance test for device /dev/video0 (not using libv4l2):
>
> Required ioctls:
> test VIDIOC_QUERYCAP: OK
>
> Allow for multiple opens:
> test second video open: OK
> test VIDIOC_QUERYCAP: OK
> test VIDIOC_G/S_PRIORITY: OK
> test for unlimited opens: OK
>
> Debug ioctls:
> test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
> test VIDIOC_LOG_STATUS: OK (Not Supported)
>
> Input ioctls:
> test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
> test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
> test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
> test VIDIOC_ENUMAUDIO: OK (Not Supported)
> test VIDIOC_G/S/ENUMINPUT: OK
> test VIDIOC_G/S_AUDIO: OK (Not Supported)
> Inputs: 1 Audio Inputs: 0 Tuners: 0
>
> Output ioctls:
> test VIDIOC_G/S_MODULATOR: OK (Not Supported)
> test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
> test VIDIOC_ENUMAUDOUT: OK (Not Supported)
> test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
> test VIDIOC_G/S_AUDOUT: OK (Not Supported)
> Outputs: 0 Audio Outputs: 0 Modulators: 0
>
> Input/Output configuration ioctls:
> test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
> test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
> test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
> test VIDIOC_G/S_EDID: OK (Not Supported)
>
> Test input 0:
>
> Control ioctls:
> test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK (Not Supported)
> test VIDIOC_QUERYCTRL: OK (Not Supported)
> test VIDIOC_G/S_CTRL: OK (Not Supported)
> test VIDIOC_G/S/TRY_EXT_CTRLS: OK (Not Supported)
> test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK (Not Supported)
> test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
> Standard Controls: 0 Private Controls: 0
I'm not sure if your driver passes control queries to the subdev. It
did not originally, and I'm not sure you picked up the change from my
version of the driver. "Not supported" here seems to indicate that it
does not.
I'd be interested what's the recommended practice here. It sure helps
with some apps that expect to be able to modify various input controls
directly on the /dev/video# device. These are then supported out of the
box.
It's a one-line change. See:
https://www.kernel.org/doc/html/latest/media/kapi/v4l2-controls.html#in
heriting-controls
> Format ioctls:
> test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
> test VIDIOC_G/S_PARM: OK (Not Supported)
> test VIDIOC_G_FBUF: OK (Not Supported)
> test VIDIOC_G_FMT: OK
> test VIDIOC_TRY_FMT: OK
> test VIDIOC_S_FMT: OK
> test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
> test Cropping: OK (Not Supported)
> test Composing: OK (Not Supported)
> test Scaling: OK (Not Supported)
>
> Codec ioctls:
> test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
> test VIDIOC_G_ENC_INDEX: OK (Not Supported)
> test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
>
> Buffer ioctls:
> test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
> test VIDIOC_EXPBUF: OK
>
> Test input 0:
>
> Streaming ioctls:
> test read/write: OK (Not Supported)
> test MMAP: OK
> test USERPTR: OK (Not Supported)
> test DMABUF: Cannot test, specify --expbuf-device
>
> Stream using all formats:
> test MMAP for Format HM12, Frame Size 1280x720:
> Stride 1920, Field None: OK
> test MMAP for Format NV12, Frame Size 1280x720:
> Stride 1920, Field None: OK
> test MMAP for Format NV21, Frame Size 1280x720:
> Stride 1920, Field None: OK
> test MMAP for Format YU12, Frame Size 1280x720:
> Stride 1920, Field None: OK
> test MMAP for Format YV12, Frame Size 1280x720:
> Stride 1920, Field None: OK
> test MMAP for Format NV16, Frame Size 1280x720:
> Stride 2560, Field None: OK
> test MMAP for Format NV61, Frame Size 1280x720:
> Stride 2560, Field None: OK
> test MMAP for Format 422P, Frame Size 1280x720:
> Stride 2560, Field None: OK
>
> Total: 54, Succeeded: 54, Failed: 0, Warnings: 0
>
> Yong Deng (2):
> dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI)
> media: V3s: Add support for Allwinner CSI.
>
> .../devicetree/bindings/media/sun6i-csi.txt | 51 ++
> MAINTAINERS | 8 +
> drivers/media/platform/Kconfig | 1 +
> drivers/media/platform/Makefile | 2 +
> drivers/media/platform/sunxi/sun6i-csi/Kconfig | 9 +
> drivers/media/platform/sunxi/sun6i-csi/Makefile | 3 +
> drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c | 878 +++++++++++++++++++++
> drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h | 147 ++++
> .../media/platform/sunxi/sun6i-csi/sun6i_csi_reg.h | 203 +++++
> .../media/platform/sunxi/sun6i-csi/sun6i_video.c | 752 ++++++++++++++++++
> .../media/platform/sunxi/sun6i-csi/sun6i_video.h | 60 ++
> 11 files changed, 2114 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt
> create mode 100644 drivers/media/platform/sunxi/sun6i-csi/Kconfig
> create mode 100644 drivers/media/platform/sunxi/sun6i-csi/Makefile
> create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h
> create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_reg.h
> create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
> create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_video.h
>
> --
> 1.8.3.1
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20171222/b8debf28/attachment-0001.sig>
^ permalink raw reply
* [PATCH] clk: divider: fix incorrect usage of container_of
From: Sylvain Lemieux @ 2017-12-22 13:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171222103419.GB18255@piout.net>
For lpc32xx:
Acked-by: Sylvain Lemieux <slemieux.tyco@gmail.com>
On Fri, Dec 22, 2017 at 5:34 AM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> On 21/12/2017 at 17:30:54 +0100, Jerome Brunet wrote:
>> divider_recalc_rate() is an helper function used by clock divider of
>> different types, so the structure containing the 'hw' pointer is not
>> always a 'struct clk_divider'
>>
>> At the following line:
>> > div = _get_div(table, val, flags, divider->width);
>>
>> in several cases, the value of 'divider->width' is garbage as the actual
>> structure behind this memory is not a 'struct clk_divider'
>>
>> Fortunately, this width value is used by _get_val() only when
>> CLK_DIVIDER_MAX_AT_ZERO flag is set. This has never been the case so
>> far when the structure is not a 'struct clk_divider'. This is probably
>> why we did not notice this bug before
>>
>> Fixes: afe76c8fd030 ("clk: allow a clk divider with max divisor when zero")
>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>
> For RTC:
> Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>
>> ---
>> Hi Stephen, Mike,
>>
>> In addition to clock, this patch also touch the rtc and drm directories.
>> As it is changing the API of the helper function, I have this fix in a
>> single commit to avoid breaking bisect.
>>
>> Please let me know if you prefer to do differently.
>>
>> Cheers
>> Jerome
>>
>> drivers/clk/clk-divider.c | 7 +++----
>> drivers/clk/hisilicon/clkdivider-hi6220.c | 2 +-
>> drivers/clk/nxp/clk-lpc32xx.c | 2 +-
>> drivers/clk/qcom/clk-regmap-divider.c | 2 +-
>> drivers/clk/sunxi-ng/ccu_div.c | 2 +-
>> drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c | 2 +-
>> drivers/rtc/rtc-ac100.c | 6 ++++--
>> include/linux/clk-provider.h | 2 +-
>> 8 files changed, 13 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
>> index 4ed516cb7276..b49942b9fe50 100644
>> --- a/drivers/clk/clk-divider.c
>> +++ b/drivers/clk/clk-divider.c
>> @@ -118,12 +118,11 @@ static unsigned int _get_val(const struct clk_div_table *table,
>> unsigned long divider_recalc_rate(struct clk_hw *hw, unsigned long parent_rate,
>> unsigned int val,
>> const struct clk_div_table *table,
>> - unsigned long flags)
>> + unsigned long flags, unsigned long width)
>> {
>> - struct clk_divider *divider = to_clk_divider(hw);
>> unsigned int div;
>>
>> - div = _get_div(table, val, flags, divider->width);
>> + div = _get_div(table, val, flags, width);
>> if (!div) {
>> WARN(!(flags & CLK_DIVIDER_ALLOW_ZERO),
>> "%s: Zero divisor and CLK_DIVIDER_ALLOW_ZERO not set\n",
>> @@ -145,7 +144,7 @@ static unsigned long clk_divider_recalc_rate(struct clk_hw *hw,
>> val &= div_mask(divider->width);
>>
>> return divider_recalc_rate(hw, parent_rate, val, divider->table,
>> - divider->flags);
>> + divider->flags, divider->width);
>> }
>>
>> static bool _is_valid_table_div(const struct clk_div_table *table,
>> diff --git a/drivers/clk/hisilicon/clkdivider-hi6220.c b/drivers/clk/hisilicon/clkdivider-hi6220.c
>> index a1c1f684ad58..9f46cf9dcc65 100644
>> --- a/drivers/clk/hisilicon/clkdivider-hi6220.c
>> +++ b/drivers/clk/hisilicon/clkdivider-hi6220.c
>> @@ -56,7 +56,7 @@ static unsigned long hi6220_clkdiv_recalc_rate(struct clk_hw *hw,
>> val &= div_mask(dclk->width);
>>
>> return divider_recalc_rate(hw, parent_rate, val, dclk->table,
>> - CLK_DIVIDER_ROUND_CLOSEST);
>> + CLK_DIVIDER_ROUND_CLOSEST, dclk->width);
>> }
>>
>> static long hi6220_clkdiv_round_rate(struct clk_hw *hw, unsigned long rate,
>> diff --git a/drivers/clk/nxp/clk-lpc32xx.c b/drivers/clk/nxp/clk-lpc32xx.c
>> index b669a5c10fee..f5d815f577e0 100644
>> --- a/drivers/clk/nxp/clk-lpc32xx.c
>> +++ b/drivers/clk/nxp/clk-lpc32xx.c
>> @@ -956,7 +956,7 @@ static unsigned long clk_divider_recalc_rate(struct clk_hw *hw,
>> val &= div_mask(divider->width);
>>
>> return divider_recalc_rate(hw, parent_rate, val, divider->table,
>> - divider->flags);
>> + divider->flags, divider->width);
>> }
>>
>> static long clk_divider_round_rate(struct clk_hw *hw, unsigned long rate,
>> diff --git a/drivers/clk/qcom/clk-regmap-divider.c b/drivers/clk/qcom/clk-regmap-divider.c
>> index 53484912301e..928fcc16ee27 100644
>> --- a/drivers/clk/qcom/clk-regmap-divider.c
>> +++ b/drivers/clk/qcom/clk-regmap-divider.c
>> @@ -59,7 +59,7 @@ static unsigned long div_recalc_rate(struct clk_hw *hw,
>> div &= BIT(divider->width) - 1;
>>
>> return divider_recalc_rate(hw, parent_rate, div, NULL,
>> - CLK_DIVIDER_ROUND_CLOSEST);
>> + CLK_DIVIDER_ROUND_CLOSEST, divider->width);
>> }
>>
>> const struct clk_ops clk_regmap_div_ops = {
>> diff --git a/drivers/clk/sunxi-ng/ccu_div.c b/drivers/clk/sunxi-ng/ccu_div.c
>> index baa3cf96507b..302a18efd39f 100644
>> --- a/drivers/clk/sunxi-ng/ccu_div.c
>> +++ b/drivers/clk/sunxi-ng/ccu_div.c
>> @@ -71,7 +71,7 @@ static unsigned long ccu_div_recalc_rate(struct clk_hw *hw,
>> parent_rate);
>>
>> val = divider_recalc_rate(hw, parent_rate, val, cd->div.table,
>> - cd->div.flags);
>> + cd->div.flags, cd->div.width);
>>
>> if (cd->common.features & CCU_FEATURE_FIXED_POSTDIV)
>> val /= cd->fixed_post_div;
>> diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c
>> index fe15aa64086f..71fe60e5f01f 100644
>> --- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c
>> +++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c
>> @@ -698,7 +698,7 @@ static unsigned long dsi_pll_14nm_postdiv_recalc_rate(struct clk_hw *hw,
>> val &= div_mask(width);
>>
>> return divider_recalc_rate(hw, parent_rate, val, NULL,
>> - postdiv->flags);
>> + postdiv->flags, width);
>> }
>>
>> static long dsi_pll_14nm_postdiv_round_rate(struct clk_hw *hw,
>> diff --git a/drivers/rtc/rtc-ac100.c b/drivers/rtc/rtc-ac100.c
>> index 9e336184491c..0282ccc6181c 100644
>> --- a/drivers/rtc/rtc-ac100.c
>> +++ b/drivers/rtc/rtc-ac100.c
>> @@ -137,13 +137,15 @@ static unsigned long ac100_clkout_recalc_rate(struct clk_hw *hw,
>> div = (reg >> AC100_CLKOUT_PRE_DIV_SHIFT) &
>> ((1 << AC100_CLKOUT_PRE_DIV_WIDTH) - 1);
>> prate = divider_recalc_rate(hw, prate, div,
>> - ac100_clkout_prediv, 0);
>> + ac100_clkout_prediv, 0,
>> + AC100_CLKOUT_PRE_DIV_WIDTH);
>> }
>>
>> div = (reg >> AC100_CLKOUT_DIV_SHIFT) &
>> (BIT(AC100_CLKOUT_DIV_WIDTH) - 1);
>> return divider_recalc_rate(hw, prate, div, NULL,
>> - CLK_DIVIDER_POWER_OF_TWO);
>> + CLK_DIVIDER_POWER_OF_TWO,
>> + AC100_CLKOUT_DIV_WIDTH);
>> }
>>
>> static long ac100_clkout_round_rate(struct clk_hw *hw, unsigned long rate,
>> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
>> index 73ac87f34df9..4c4001086447 100644
>> --- a/include/linux/clk-provider.h
>> +++ b/include/linux/clk-provider.h
>> @@ -412,7 +412,7 @@ extern const struct clk_ops clk_divider_ro_ops;
>>
>> unsigned long divider_recalc_rate(struct clk_hw *hw, unsigned long parent_rate,
>> unsigned int val, const struct clk_div_table *table,
>> - unsigned long flags);
>> + unsigned long flags, unsigned long width);
>> long divider_round_rate_parent(struct clk_hw *hw, struct clk_hw *parent,
>> unsigned long rate, unsigned long *prate,
>> const struct clk_div_table *table,
>> --
>> 2.14.3
>>
>
> --
> Alexandre Belloni, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
^ permalink raw reply
* [PATCH 0/4] vf610-zii-dev updates
From: Russell King - ARM Linux @ 2017-12-22 14:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171220231108.GJ10595@n2100.armlinux.org.uk>
On Wed, Dec 20, 2017 at 11:11:08PM +0000, Russell King - ARM Linux wrote:
> Hi,
>
> These patches update the DT for the ZII VF610 boards.
>
> The first patch fixes complaints at boot about missing DMAs on rev C
> boards, particularly for the SPI interface. This is because edma1 is
> not enabled. This seems to be a regression from the 4.10 era.
>
> The second patch fixes an interrupt storm during boot on rev B boards,
> which causes boot to take 80+ seconds - this seems to be a long
> standing issue since the DT description was first added. The PTB28
> pin is definitely GPIO 98, and GPIO 98 is definitely part of the
> gpio3 block, not the gpio2 block. Since GPIO 66 (which is the
> corresponding GPIO in gpio2) is low, and the IRQ trigger is level-low,
> this causes an interrupt storm.
>
> The last two patches add an explicit description of the PHYs that are
> actually connected to the switch - the 88e1545 is a quad PHY, and
> without describing the MDIO bus, DSA assumes that any PHYs it can
> discover are present for the switch. As only the first three PHYs
> are connected, this leads the 4th port to believe it is connected to
> the 4th PHY when the fixed-link definition is (eventually) removed.
>
> Head this off by providing the proper descriptions, and as we have
> them, also describe the interrupts for these PHYs.
>
> Note, however, that the interrupt description is not quite correct -
> the 88e1545 PHYs all share one interrupt line, and there is a register
> in the PHY which can be used to demux the interrupt to the specific
> PHY. However, in this description, we ignore the demux register, and
> just share the interrupt between the PHYs. That much is fine, but
> the pinmuxing becomes problematical - if we describe the same pinmux
> settings for each PHY for the interrupt line, the 2nd/3rd PHYs fail.
> This has no known solution. Suggestions welcome.
>
> arch/arm/boot/dts/vf610-zii-dev-rev-b.dts | 34 ++++++++++++++++++++++++++++++-
> arch/arm/boot/dts/vf610-zii-dev.dtsi | 4 ++++
> 2 files changed, 37 insertions(+), 1 deletion(-)
There's more stuff that isn't correct in the Vybrid DTS files...
When the SoC was first added, the vfxxx.dtsi had:
+ adc0: adc at 4003b000 {
...
+ status = "disabled";
+ };
+ adc1: adc at 400bb000 {
...
+ status = "disabled";
+ };
This default status remains today.
IIO hwmon support was added later:
+ iio-hwmon {
+ compatible = "iio-hwmon";
+ io-channels = <&adc0 16>, <&adc1 16>;
+ };
which is all fine and dandy, but iio-hwmon fails to probe unless it can
find _all_ the io-channels specified.
Given that the two ADC channels referenced by iio-hwmon default to being
disabled, it makes no sense for iio-hwmon to default to being enabled.
What's more is that if, say, adc0 is enabled by a board, the iio-hwmon
device still fails to be probed because it fails to get adc1.
As I see it, there's two possible solutions to this:
1. remove the default disabled status of the ADCs so both are always
available, or
2. default iio-hwmon to disabled, and provide a label for this device
so that a correct io-channels specification can be suppled for the
board in question, and for iio-hwmon to be enabled where appropriate.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply
* [PATCH v2] arm64: dts: Hi3660: Fix up psci state id
From: Vincent Guittot @ 2017-12-22 14:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1513069954-22827-1-git-send-email-leo.yan@linaro.org>
Hi Leo,
Sorry for jumping late in the discussion but should we also remove
the NAP state from the property cpu-idle-states of the CPUs because
this state not supported by the platform at least for now and may be
not in a near future ?
Then, I have another question regarding the update of the
psci-suspend-parameter. These changes implies an update of the psci
firmawre which means that we will now have 2 different firmware
version compatible with 2 different dt.
Is there any way to check that the ATF on the board is the one that
compatible with the parameter with something like a version ? I
currently use the previous firmware which works fine with current
kernel and dt binding once the NAP state is removed from the table.
When moving on recent kernel, I will have to take care of updating the
firmware and if i need to go back on a previous kernel, i will have to
make sure that i have the right ATF version. This make a lot of chance
of having the wrong configuration
Regards,
Vincent
On 12 December 2017 at 10:12, Leo Yan <leo.yan@linaro.org> wrote:
> Thanks a lot for Vincent Guittot careful work to find bug for 'CPU_NAP'
> idle state. From ftrace log we can observe CA73 CPUs can be easily
> waken up from 'CPU_NAP' state but the 'waken up' CPUs doesn't handle
> anything and sleep again; so there have tons of trace events for CA73
> CPUs entering and exiting idle state.
>
> On Hi3660 CA73 has retention state 'CPU_NAP' for CPU idle, this state we
> set its psci parameter as '0x0000001' and from this parameter it can
> calculate state id is 1. Unfortunately ARM trusted firmware (ARM-TF)
> takes 1 as a invalid value for state id, so the CPU cannot enter idle
> state and directly bail out to kernel.
>
> We want to create good practice for psci parameters platform definition,
> so review the psci specification. The spec "ARM Power State Coordination
> Interface - Platform Design Document (ARM DEN 0022D)" recommends state
> ID in chapter "6.5 Recommended StateID Encoding". The recommended power
> state IDs can be presented by below listed values; and it divides into
> three fields, every field can use 4 bits to present power states
> corresponding to core level, cluster level and system level:
> 0: Run
> 1: Standby
> 2: Retention
> 3: Powerdown
>
> This commit changes psci parameter to compliance with the suggested
> state ID in the doc. Except we change 'CPU_NAP' state psci parameter
> to '0x0000002', this commit also changes 'CPU_SLEEP' and 'CLUSTER_SLEEP'
> state parameters to '0x0010003' and '0x1010033' respectively.
>
> Credits to Daniel, Sudeep and Soby for suggestion and consolidation.
>
> Cc: Vincent Guittot <vincent.guittot@linaro.org>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Soby Mathew <Soby.Mathew@arm.com>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
> arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> index ab0b95b..99d5a46 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> @@ -147,7 +147,7 @@
>
> CPU_NAP: cpu-nap {
> compatible = "arm,idle-state";
> - arm,psci-suspend-param = <0x0000001>;
> + arm,psci-suspend-param = <0x0000002>;
> entry-latency-us = <7>;
> exit-latency-us = <2>;
> min-residency-us = <15>;
> @@ -156,7 +156,7 @@
> CPU_SLEEP: cpu-sleep {
> compatible = "arm,idle-state";
> local-timer-stop;
> - arm,psci-suspend-param = <0x0010000>;
> + arm,psci-suspend-param = <0x0010003>;
> entry-latency-us = <40>;
> exit-latency-us = <70>;
> min-residency-us = <3000>;
> @@ -165,7 +165,7 @@
> CLUSTER_SLEEP_0: cluster-sleep-0 {
> compatible = "arm,idle-state";
> local-timer-stop;
> - arm,psci-suspend-param = <0x1010000>;
> + arm,psci-suspend-param = <0x1010033>;
> entry-latency-us = <500>;
> exit-latency-us = <5000>;
> min-residency-us = <20000>;
> @@ -174,7 +174,7 @@
> CLUSTER_SLEEP_1: cluster-sleep-1 {
> compatible = "arm,idle-state";
> local-timer-stop;
> - arm,psci-suspend-param = <0x1010000>;
> + arm,psci-suspend-param = <0x1010033>;
> entry-latency-us = <1000>;
> exit-latency-us = <5000>;
> min-residency-us = <20000>;
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH] dt: psci: Update DT bindings to support hierarchical PSCI states
From: Ulf Hansson @ 2017-12-22 14:32 UTC (permalink / raw)
To: linux-arm-kernel
From: Lina Iyer <lina.iyer@linaro.org>
Update DT bindings to represent hierarchical CPU and CPU domain idle states
for PSCI. Also update the PSCI examples to clearly show how flattened and
hierarchical idle states can be represented in DT.
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
For your information, I have picked up the work from Lina Iyer around the so
called CPU cluster idling series [1,2] and I working on new versions. However,
I decided to post the updates to the PSCI DT bindings first, as they will be
needed to be agreed upon before further changes can be done to the PSCI firmware
driver.
Note, these bindings have been discussed over and over again, at LKML, but
especially also at various Linux conferences, like LPC and Linaro Connect. We
finally came to a conclusion and the changes we agreed upon, should be reflected
in this update.
Of course, it's a while ago since the latest discussions, but hopefully people
don't have too hard time to remember.
Kind regards
Uffe
[1]
https://www.spinics.net/lists/arm-kernel/msg566200.html
[2]
https://lwn.net/Articles/716300/
---
Documentation/devicetree/bindings/arm/psci.txt | 152 +++++++++++++++++++++++++
1 file changed, 152 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/psci.txt b/Documentation/devicetree/bindings/arm/psci.txt
index a2c4f1d..5a8f11b 100644
--- a/Documentation/devicetree/bindings/arm/psci.txt
+++ b/Documentation/devicetree/bindings/arm/psci.txt
@@ -105,7 +105,159 @@ Case 3: PSCI v0.2 and PSCI v0.1.
...
};
+PSCI v1.0 onwards, supports OS-Initiated mode for powering off CPUs and CPU
+clusters from the firmware. For such topologies the PSCI firmware driver acts
+as pseudo-controller, which may be specified in the psci DT node. The
+definitions of the CPU and the CPU cluster topology, must conform to the domain
+idle state specification [3]. The domain idle states themselves, must be
+compatible with the defined 'domain-idle-state' binding [1], and also need to
+specify the arm,psci-suspend-param property for each idle state.
+
+DT allows representing CPU and CPU cluster idle states in two different ways -
+
+The flattened model as given in Example 1, lists CPU's idle states followed by
+the domain idle state that the CPUs may choose. This is the general practice
+followed in PSCI firmwares that support Platform Coordinated mode. Note that
+the idle states are all compatible with "arm,idle-state".
+
+Example 2 represents the hierarchical model of CPU and domain idle states.
+CPUs define their domain provider in their DT node. The domain controls the
+power to the CPU and possibly other h/w blocks that would be powered off when
+the CPU is powered off. The CPU's idle states may therefore be considered as
+the domain's idle states and have the compatible "arm,idle-state". Such domains
+may be embedded within another domain that represents common h/w blocks between
+these CPUs viz. the cluster. The idle states of the cluster would be
+represented as the domain's idle states. In order to use OS-Initiated mode of
+PSCI in the firmware, the hierarchical representation must be used.
+
+Example 1: Flattened representation of CPU and domain idle states
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ CPU0: cpu at 0 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53", "arm,armv8";
+ reg = <0x0>;
+ enable-method = "psci";
+ cpu-idle-states = <&CPU_PWRDN>, <&CLUSTER_RET>,
+ <&CLUSTER_PWR_DWN>;
+ };
+
+ CPU1: cpu at 1 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a57", "arm,armv8";
+ reg = <0x100>;
+ enable-method = "psci";
+ cpu-idle-states = <&CPU_PWRDN>, <&CLUSTER_RET>,
+ <&CLUSTER_PWR_DWN>;
+ };
+
+ idle-states {
+ CPU_PWRDN: cpu_power_down{
+ compatible = "arm,idle-state";
+ arm,psci-suspend-param = <0x000001>;
+ entry-latency-us = <10>;
+ exit-latency-us = <10>;
+ min-residency-us = <100>;
+ };
+
+ CLUSTER_RET: domain_ret {
+ compatible = "arm,idle-state";
+ arm,psci-suspend-param = <0x1000010>;
+ entry-latency-us = <500>;
+ exit-latency-us = <500>;
+ min-residency-us = <2000>;
+ };
+
+ CLUSTER_PWR_DWN: domain_off {
+ compatible = "arm,idle-state";
+ arm,psci-suspend-param = <0x1000030>;
+ entry-latency-us = <2000>;
+ exit-latency-us = <2000>;
+ min-residency-us = <6000>;
+ };
+ };
+
+ psci {
+ compatible = "arm,psci-0.2";
+ method = "smc";
+ };
+
+Example 2: Hierarchical representation of CPU and domain idle states
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ CPU0: cpu at 0 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53", "arm,armv8";
+ reg = <0x0>;
+ enable-method = "psci";
+ power-domains = <&CPU_PD0>;
+ };
+
+ CPU1: cpu at 1 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a57", "arm,armv8";
+ reg = <0x100>;
+ enable-method = "psci";
+ power-domains = <&CPU_PD1>;
+ };
+
+ idle-states {
+ CPU_PWRDN: cpu_power_down{
+ compatible = "arm,idle-state";
+ arm,psci-suspend-param = <0x000001>;
+ entry-latency-us = <10>;
+ exit-latency-us = <10>;
+ min-residency-us = <100>;
+ };
+
+ CLUSTER_RET: domain_ret {
+ compatible = "domain-idle-state";
+ arm,psci-suspend-param = <0x1000010>;
+ entry-latency-us = <500>;
+ exit-latency-us = <500>;
+ min-residency-us = <2000>;
+ };
+
+ CLUSTER_PWR_DWN: domain_off {
+ compatible = "domain-idle-state";
+ arm,psci-suspend-param = <0x1000030>;
+ entry-latency-us = <2000>;
+ exit-latency-us = <2000>;
+ min-residency-us = <6000>;
+ };
+ };
+ };
+
+ psci {
+ compatible = "arm,psci-1.0";
+ method = "smc";
+
+ CPU_PD0: cpu-pd at 0 {
+ #power-domain-cells = <0>;
+ domain-idle-states = <&CPU_PWRDN>;
+ power-domains = <&CLUSTER_PD>;
+ };
+
+ CPU_PD1: cpu-pd at 1 {
+ #power-domain-cells = <0>;
+ domain-idle-states = <&CPU_PWRDN>;
+ power-domains = <&CLUSTER_PD>;
+ };
+
+ CLUSTER_PD: cluster-pd at 0 {
+ #power-domain-cells = <0>;
+ domain-idle-states = <&CLUSTER_RET>, <&CLUSTER_PWR_DWN>;
+ };
+ };
+
[1] Kernel documentation - ARM idle states bindings
Documentation/devicetree/bindings/arm/idle-states.txt
[2] Power State Coordination Interface (PSCI) specification
http://infocenter.arm.com/help/topic/com.arm.doc.den0022c/DEN0022C_Power_State_Coordination_Interface.pdf
+[3]. PM Domains description
+ Documentation/devicetree/bindings/power/power_domain.txt
--
2.7.4
^ permalink raw reply related
* [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver
From: H. Nikolaus Schaller @ 2017-12-22 14:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171222124427.GI3374@localhost>
Hi Johan,
> Am 22.12.2017 um 13:44 schrieb Johan Hovold <johan@kernel.org>:
>
> On Fri, Dec 01, 2017 at 08:49:36AM +0100, H. Nikolaus Schaller wrote:
>> Add driver for Wi2Wi W2SG0004/84 GPS module connected to some SoC UART.
>>
>> It uses serdev API hooks to monitor and forward the UART traffic to /dev/ttyGPSn
>> and turn on/off the module. It also detects if the module is turned on (sends data)
>> but should be off, e.g. if it was already turned on during boot or power-on-reset.
>>
>> Additionally, rfkill block/unblock can be used to control an external LNA
>> (and power down the module if not needed).
>>
>> The driver concept is based on code developed by Neil Brown <neilb@suse.de>
>> but simplified and adapted to use the new serdev API introduced in v4.11.
>
> I'm sorry (and I know this discussion has been going on for a long
> time),
I'd say: already too much time.
> but this still feels like too much of a hack.
Well, to me it feels like review process is trying to get things 200% right
in the first step and therefore delays proposals that may already be useful
to real world users. Review is of course important to protect against severe
bugs and security issues.
My concern is that in 5 more years this chip is obsolete and then we never
had a working solution in distributions that base directly on kernel.org...
What I am lacking in this discussion is the spirit of starting with something
that basically works and permanently enhancing things. Like Linux started
25 years ago and many drivers look very different in v4.15 than v2.6.32.
Instead, we are sent back every time to rework it completely and at some point
our enthusiasm (and time budget) has boiled down to 0 because we see no more
reward for working on this.
>
> You're registering a tty driver to allow user space to continue treat
> this as a tty device, but you provide no means of actually modifying
> anything (line settings, etc).
That was planned for a second step but is not important for pure GPS
reception.
> It's essentially just a character device
> with common tty ioctls as noops from a device PoV (well, plus the ldisc
> buffering and processing).
Yes. The buffering is the more important aspect and as far as I understand
we would have to implement our own buffer for a chardev driver. Buffering is
perfectly solved for tty devices.
Another aspect is that the same chip connected through an USB or Bluetooth
connection is presented as a tty device and not a char dev.
> This will probably require someone to first implement a generic gps
> framework with a properly defined interface which you may then teach
> gpsd to use (e.g. to avoid all its autodetection functionality) instead.
Yes, that is what I dream of.
But in past 5 years there was no "someone" to pop up and my time to work
on this topic is too limited. It is not even my main focus of efforts.
So I prefer solutions that can be done today with today's means and not
dreams (even if we share them).
> Or some entirely different approach, for example, where you manage
> everything from user space.
>
> I'd suggest reiterating the problem you're trying to solve and
> enumerating the previously discussed potential solutions in order to
> find a proper abstraction level for this (before getting lost in
> implementation details).
Yes, please feel free to write patches that implement it that way.
>
> That being said, I'm still pointing some bugs and issue below that you
> can consider for future versions of this (and other drivers) below.
Thanks!
>
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> ---
>> drivers/misc/Kconfig | 10 +
>> drivers/misc/Makefile | 1 +
>> drivers/misc/w2sg0004.c | 553 ++++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 564 insertions(+)
>> create mode 100644 drivers/misc/w2sg0004.c
>>
>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>> index f1a5c2357b14..a3b11016ed2b 100644
>> --- a/drivers/misc/Kconfig
>> +++ b/drivers/misc/Kconfig
>> @@ -508,4 +508,14 @@ source "drivers/misc/mic/Kconfig"
>> source "drivers/misc/genwqe/Kconfig"
>> source "drivers/misc/echo/Kconfig"
>> source "drivers/misc/cxl/Kconfig"
>> +
>> +config W2SG0004
>> + tristate "W2SG00x4 on/off control"
>
> Please provide a better summary for what this driver does.
Ok.
>
>> + depends on GPIOLIB && SERIAL_DEV_BUS
>> + help
>> + Enable on/off control of W2SG00x4 GPS moduled connected
s/moduled/module/
>
> Some whitespace issue here.
Ok.
>
>> + to some SoC UART to allow powering up/down if the /dev/ttyGPSn
>> + is opened/closed.
>> + It also provides a rfkill gps name to control the LNA power.
>> +
>> endmenu
>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>> index 5ca5f64df478..d9d824b3d20a 100644
>> --- a/drivers/misc/Makefile
>> +++ b/drivers/misc/Makefile
>> @@ -50,6 +50,7 @@ obj-$(CONFIG_SRAM_EXEC) += sram-exec.o
>> obj-y += mic/
>> obj-$(CONFIG_GENWQE) += genwqe/
>> obj-$(CONFIG_ECHO) += echo/
>> +obj-$(CONFIG_W2SG0004) += w2sg0004.o
>> obj-$(CONFIG_VEXPRESS_SYSCFG) += vexpress-syscfg.o
>> obj-$(CONFIG_CXL_BASE) += cxl/
>> obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
>> diff --git a/drivers/misc/w2sg0004.c b/drivers/misc/w2sg0004.c
>> new file mode 100644
>> index 000000000000..6bfd12eb8e02
>> --- /dev/null
>> +++ b/drivers/misc/w2sg0004.c
>> @@ -0,0 +1,553 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Driver for power controlling the w2sg0004/w2sg0084 GPS receiver.
>> + *
>> + * Copyright (C) 2013 Neil Brown <neilb@suse.de>
>> + * Copyright (C) 2015-2017 H. Nikolaus Schaller <hns@goldelico.com>,
>> + * Golden Delicious Computers
>> + *
>> + * This receiver has an ON/OFF pin which must be toggled to
>> + * turn the device 'on' of 'off'. A high->low->high toggle
>
> s/of/or/
Ok.
>
>> + * will switch the device on if it is off, and off if it is on.
>> + *
>> + * To enable receiving on/off requests we register with the
>> + * UART power management notifications.
>
> No, the UART (serial device) would be the grandparent of your serdev
> device (for which you register PM callbacks).
Ah, thanks. This came from original description 5 years ago and if
you work too long on something you do no longer read what is really
written.
>
>> + *
>> + * It is not possible to directly detect the state of the device.
>
> Didn't the 0084 version have a pin for this?
AFAIR no and even if it had, it would not help for the 0004 device
(80% of the GTA04 devices have been built with the 0004 and there is no known
mechanism to detect the chip variant during runtime).
>
>> + * However when it is on it will send characters on a UART line
>> + * regularly.
>> + *
>> + * To detect that the power state is out of sync (e.g. if GPS
>> + * was enabled before a reboot), we register for UART data received
>> + * notifications.
>> + *
>> + * In addition we register as a rfkill client so that we can
>> + * control the LNA power.
>> + *
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/irq.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/of_gpio.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/rfkill.h>
>> +#include <linux/serdev.h>
>> +#include <linux/sched.h>
>> +#include <linux/slab.h>
>> +#include <linux/tty.h>
>> +#include <linux/tty_flip.h>
>> +#include <linux/workqueue.h>
>> +
>> +/*
>> + * There seems to be restrictions on how quickly we can toggle the
>> + * on/off line. data sheets says "two rtc ticks", whatever that means.
>> + * If we do it too soon it doesn't work.
>> + * So we have a state machine which uses the common work queue to ensure
>> + * clean transitions.
>> + * When a change is requested we record that request and only act on it
>> + * once the previous change has completed.
>> + * A change involves a 10ms low pulse, and a 990ms raised level, so only
>> + * one change per second.
>> + */
>> +
>> +enum w2sg_state {
>> + W2SG_IDLE, /* is not changing state */
>> + W2SG_PULSE, /* activate on/off impulse */
>> + W2SG_NOPULSE /* deactivate on/off impulse */
>> +};
>> +
>> +struct w2sg_data {
>> + struct rfkill *rf_kill;
>> + struct regulator *lna_regulator;
>> + int lna_blocked; /* rfkill block gps active */
>> + int lna_is_off; /* LNA is currently off */
>> + int is_on; /* current state (0/1) */
>
> These look like flags; use a bitfield rather than an int.
Well, yes. But what does it save? 8 bytes in memory?
If we use char it would not even save anything.
>
>> + unsigned long last_toggle;
>> + unsigned long backoff; /* time to wait since last_toggle */
>> + int on_off_gpio; /* the on-off gpio number */
>> + struct serdev_device *uart; /* uart connected to the chip */
>> + struct tty_driver *tty_drv; /* this is the user space tty */
>
> This does not belong in the device data as someone (Arnd?) already
> pointed out to you in a comment to an earlier version. More below.
>
>> + struct device *dev; /* from tty_port_register_device() */
>> + struct tty_port port;
>> + int open_count; /* how often we were opened */
>> + enum w2sg_state state;
>> + int requested; /* requested state (0/1) */
>> + int suspended;
>> + struct delayed_work work;
>> + int discard_count;
>> +};
>
> You seem to have completely ignored locking. These flags and resources
> are accessed from multiple contexts, and it all looks very susceptible
> to races (e.g. the work queue can race with the rfkill callback and
> leave the regulator enabled when it should be off).
Has never been observed in practise, but you are right to ask to consider.
>
>> +/* should become w2sg_by_minor[n] if we want to support multiple devices */
>> +static struct w2sg_data *w2sg_by_minor;
>> +
>> +static int w2sg_set_lna_power(struct w2sg_data *data)
>> +{
>> + int ret = 0;
>> + int off = data->suspended || !data->requested || data->lna_blocked;
>> +
>> + pr_debug("%s: %s\n", __func__, off ? "off" : "on");
>
> This and other pr_xxx should be replaced with dev_dbg and friends.
Ok.
>
>> +
>> + if (off != data->lna_is_off) {
>> + data->lna_is_off = off;
>> + if (!IS_ERR_OR_NULL(data->lna_regulator)) {
>> + if (off)
>> + regulator_disable(data->lna_regulator);
>> + else
>> + ret = regulator_enable(data->lna_regulator);
>> + }
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static void w2sg_set_power(void *pdata, int val)
>
> Don't pass around void pointers like this.
Agreed. There might have been a reason years ago...
Maybe it was originally a callback handler with some opaque user-data pointer.
>
>> +{
>> + struct w2sg_data *data = (struct w2sg_data *) pdata;
>> +
>> + pr_debug("%s to state=%d (requested=%d)\n", __func__, val,
>> + data->requested);
>> +
>> + if (val && !data->requested) {
>> + data->requested = true;
>> + } else if (!val && data->requested) {
>> + data->backoff = HZ;
>> + data->requested = false;
>> + } else
>> + return;
>
> Missing brackets.
Ok.
Interestingly there is no warning from checkpatch.
>
>> +
>> + if (!data->suspended)
>> + schedule_delayed_work(&data->work, 0);
>> +}
>> +
>> +/* called each time data is received by the UART (i.e. sent by the w2sg0004) */
>> +static int w2sg_uart_receive_buf(struct serdev_device *serdev,
>> + const unsigned char *rxdata,
>> + size_t count)
>> +{
>> + struct w2sg_data *data =
>> + (struct w2sg_data *) serdev_device_get_drvdata(serdev);
>> +
>> + if (!data->requested && !data->is_on) {
>> + /*
>> + * we have received characters while the w2sg
>> + * should have been be turned off
>> + */
>> + data->discard_count += count;
>> + if ((data->state == W2SG_IDLE) &&
>> + time_after(jiffies,
>> + data->last_toggle + data->backoff)) {
>> + /* Should be off by now, time to toggle again */
>> + pr_debug("w2sg00x4 has sent %d characters data although it should be off!\n",
>> + data->discard_count);
>> +
>> + data->discard_count = 0;
>> +
>> + data->is_on = true;
>> + data->backoff *= 2;
>> + if (!data->suspended)
>> + schedule_delayed_work(&data->work, 0);
>> + }
>> + } else if (data->open_count > 0) {
>> + int n;
>> +
>> + pr_debug("w2sg00x4: push %lu chars to tty port\n",
>> + (unsigned long) count);
>> +
>> + /* pass to user-space */
>> + n = tty_insert_flip_string(&data->port, rxdata, count);
>> + if (n != count)
>> + pr_err("w2sg00x4: did loose %lu characters\n",
>> + (unsigned long) (count - n));
>> + tty_flip_buffer_push(&data->port);
>> + return n;
>> + }
>> +
>> + /* assume we have processed everything */
>> + return count;
>> +}
>> +
>> +/* try to toggle the power state by sending a pulse to the on-off GPIO */
>> +static void toggle_work(struct work_struct *work)
>> +{
>> + struct w2sg_data *data = container_of(work, struct w2sg_data,
>> + work.work);
>> +
>> + switch (data->state) {
>> + case W2SG_IDLE:
>> + if (data->requested == data->is_on)
>> + return;
>> +
>> + w2sg_set_lna_power(data); /* update LNA power state */
>> + gpio_set_value_cansleep(data->on_off_gpio, 0);
>> + data->state = W2SG_PULSE;
>> +
>> + pr_debug("w2sg: power gpio ON\n");
>> +
>> + schedule_delayed_work(&data->work,
>> + msecs_to_jiffies(10));
>> + break;
>> +
>> + case W2SG_PULSE:
>> + gpio_set_value_cansleep(data->on_off_gpio, 1);
>> + data->last_toggle = jiffies;
>> + data->state = W2SG_NOPULSE;
>> + data->is_on = !data->is_on;
>> +
>> + pr_debug("w2sg: power gpio OFF\n");
>> +
>> + schedule_delayed_work(&data->work,
>> + msecs_to_jiffies(10));
>> + break;
>> +
>> + case W2SG_NOPULSE:
>> + data->state = W2SG_IDLE;
>> +
>> + pr_debug("w2sg: idle\n");
>> +
>> + break;
>> +
>> + }
>> +}
>> +
>> +static int w2sg_rfkill_set_block(void *pdata, bool blocked)
>> +{
>> + struct w2sg_data *data = pdata;
>> +
>> + pr_debug("%s: blocked: %d\n", __func__, blocked);
>> +
>> + data->lna_blocked = blocked;
>> +
>> + return w2sg_set_lna_power(data);
>> +}
>> +
>> +static struct rfkill_ops w2sg0004_rfkill_ops = {
>> + .set_block = w2sg_rfkill_set_block,
>> +};
>> +
>> +static struct serdev_device_ops serdev_ops = {
>> + .receive_buf = w2sg_uart_receive_buf,
>> +};
>> +
>> +/*
>> + * we are a man-in the middle between the user-space visible tty port
>> + * and the serdev tty where the chip is connected.
>> + * This allows us to recognise when the device should be powered on
>> + * or off and handle the "false" state that data arrives while no
>> + * users-space tty client exists.
>> + */
>> +
>> +static struct w2sg_data *w2sg_get_by_minor(unsigned int minor)
>> +{
>> + BUG_ON(minor >= 1);
>
> Never use BUG_ON in driver code.
Ok.
>
>> + return w2sg_by_minor;
>> +}
>> +
>> +static int w2sg_tty_install(struct tty_driver *driver, struct tty_struct *tty)
>> +{
>> + struct w2sg_data *data;
>> + int retval;
>> +
>> + pr_debug("%s() tty = %p\n", __func__, tty);
>> +
>> + data = w2sg_get_by_minor(tty->index);
>> +
>> + if (!data)
>> + return -ENODEV;
>> +
>> + retval = tty_standard_install(driver, tty);
>> + if (retval)
>> + goto error_init_termios;
>> +
>> + tty->driver_data = data;
>> +
>> + return 0;
>> +
>> +error_init_termios:
>> + tty_port_put(&data->port);
>
> Where's the corresponding get?
I think we copied gb_tty_install or something alike
http://elixir.free-electrons.com/linux/v4.15-rc4/source/drivers/staging/greybus/uart.c#L393
but that might be buggy or we didn't copy all (tty_port-get) what is needed to make it correct.
>
>> + return retval;
>> +}
>> +
>> +static int w2sg_tty_open(struct tty_struct *tty, struct file *file)
>> +{
>> + struct w2sg_data *data = tty->driver_data;
>> +
>> + pr_debug("%s() data = %p open_count = ++%d\n", __func__,
>> + data, data->open_count);
>> +
>> + w2sg_set_power(data, ++data->open_count > 0);
>> +
>> + return tty_port_open(&data->port, tty, file);
>
> You'd leave the open count incremented on errors here.
Ok.
>
>> +}
>> +
>> +static void w2sg_tty_close(struct tty_struct *tty, struct file *file)
>> +{
>> + struct w2sg_data *data = tty->driver_data;
>> +
>> + pr_debug("%s()\n", __func__);
>> +
>> + w2sg_set_power(data, --data->open_count > 0);
>> +
>> + tty_port_close(&data->port, tty, file);
>> +}
>> +
>> +static int w2sg_tty_write(struct tty_struct *tty,
>> + const unsigned char *buffer, int count)
>> +{
>> + struct w2sg_data *data = tty->driver_data;
>> +
>> + /* simply pass down to UART */
>> + return serdev_device_write_buf(data->uart, buffer, count);
>> +}
>> +
>> +static const struct tty_operations w2sg_serial_ops = {
>> + .install = w2sg_tty_install,
>> + .open = w2sg_tty_open,
>> + .close = w2sg_tty_close,
>> + .write = w2sg_tty_write,
>> +};
>> +
>> +static const struct tty_port_operations w2sg_port_ops = {
>> + /* none defined, but we need the struct */
>> +};
>> +
>> +static int w2sg_probe(struct serdev_device *serdev)
>> +{
>> + struct w2sg_data *data;
>> + struct rfkill *rf_kill;
>> + int err;
>> + int minor;
>> + enum of_gpio_flags flags;
>> +
>> + pr_debug("%s()\n", __func__);
>> +
>> + /*
>> + * For future consideration:
>> + * for multiple such GPS receivers in one system
>> + * we need a mechanism to define distinct minor values
>> + * and search for an unused one.
>> + */
>> + minor = 0;
>> + if (w2sg_get_by_minor(minor)) {
>> + pr_err("w2sg minor is already in use!\n");
>> + return -ENODEV;
>> + }
>> +
>> + data = devm_kzalloc(&serdev->dev, sizeof(*data), GFP_KERNEL);
>> + if (data == NULL)
>> + return -ENOMEM;
>> +
>> + serdev_device_set_drvdata(serdev, data);
>> +
>> + data->on_off_gpio = of_get_named_gpio_flags(serdev->dev.of_node,
>> + "enable-gpios", 0,
>> + &flags);
>
> You should be using gpio descriptors and not the legacy interface.
Ok.
>
>> + /* defer until we have all gpios */
>> + if (data->on_off_gpio == -EPROBE_DEFER)
>> + return -EPROBE_DEFER;
>> +
>> + data->lna_regulator = devm_regulator_get_optional(&serdev->dev,
>> + "lna");
>> + if (IS_ERR(data->lna_regulator)) {
>> + /* defer until we can get the regulator */
>> + if (PTR_ERR(data->lna_regulator) == -EPROBE_DEFER)
>> + return -EPROBE_DEFER;
>> +
>> + data->lna_regulator = NULL;
>> + }
>> + pr_debug("%s() lna_regulator = %p\n", __func__, data->lna_regulator);
>> +
>> + data->lna_blocked = true;
>> + data->lna_is_off = true;
>> +
>> + data->is_on = false;
>> + data->requested = false;
>> + data->state = W2SG_IDLE;
>> + data->last_toggle = jiffies;
>> + data->backoff = HZ;
>> +
>> + data->uart = serdev;
>> +
>> + INIT_DELAYED_WORK(&data->work, toggle_work);
>> +
>> + err = devm_gpio_request(&serdev->dev, data->on_off_gpio,
>> + "w2sg0004-on-off");
>> + if (err < 0)
>> + goto out;
>
> Just return unless you're actually undwinding something.
>
>> +
>> + gpio_direction_output(data->on_off_gpio, false);
>> +
>> + serdev_device_set_client_ops(data->uart, &serdev_ops);
>> + serdev_device_open(data->uart);
>
> Missing error handling.
>
> And always keeping the port open would in most cases prevent the serial
> controller from runtime suspending.
Ok.
>
>> +
>> + serdev_device_set_baudrate(data->uart, 9600);
>> + serdev_device_set_flow_control(data->uart, false);
>
> So you hardcode these settings and provide no means for user space to
> change them. This may make sense for this GPS, but it looks like at
> least some of the wi2wi 0084 modules use 4800 baud ("i"-versions?).
That is new to me that there is an i-version (we don't have it in use).
If you have such a device, please feel free to integrate a dynamic
setting depending on chip variant (e.g. based on compatible string).
>
>> +
>> + rf_kill = rfkill_alloc("GPS", &serdev->dev, RFKILL_TYPE_GPS,
>> + &w2sg0004_rfkill_ops, data);
>> + if (rf_kill == NULL) {
>> + err = -ENOMEM;
>> + goto err_rfkill;
>
> Name error labels after what they do (not where you jump from).
Ok.
>
> That may have prevented the NULL deref you'd trigger in the error path
> here.
>
>> + }
>> +
>> + err = rfkill_register(rf_kill);
>> + if (err) {
>> + dev_err(&serdev->dev, "Cannot register rfkill device\n");
>> + goto err_rfkill;
>> + }
>> +
>> + data->rf_kill = rf_kill;
>> +
>> + /* allocate the tty driver */
>> + data->tty_drv = alloc_tty_driver(1);
>
> As was already pointed out by Arnd in a review of an previous version of
> this series, this must not be done at probe (do it at module init). We
> don't want separate tty drivers for every device.
Looks as if we should use tty_alloc_driver() in addition.
>
>> + if (!data->tty_drv)
>> + return -ENOMEM;
>
> Here you'd leak the registered rfkill structure, and leave the port
> open.
Ok.
>
>> +
>> + /* initialize the tty driver */
>> + data->tty_drv->owner = THIS_MODULE;
>> + data->tty_drv->driver_name = "w2sg0004";
>> + data->tty_drv->name = "ttyGPS";
>
> I don't think you should be claiming this generic namespace for
> something as specific as this.
Suggestion?
>
>> + data->tty_drv->major = 0;
>> + data->tty_drv->minor_start = 0;
>> + data->tty_drv->type = TTY_DRIVER_TYPE_SERIAL;
>> + data->tty_drv->subtype = SERIAL_TYPE_NORMAL;
>> + data->tty_drv->flags = TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV;
>> + data->tty_drv->init_termios = tty_std_termios;
>> + data->tty_drv->init_termios.c_cflag = B9600 | CS8 | CREAD |
>> + HUPCL | CLOCAL;
>> + /*
>> + * optional:
>> + * tty_termios_encode_baud_rate(&data->tty_drv->init_termios,
>> + 115200, 115200);
>> + * w2sg_tty_termios(&data->tty_drv->init_termios);
>> + */
>
> Why is this here?
Was planned to integrate in a second step.
>
>> + tty_set_operations(data->tty_drv, &w2sg_serial_ops);
>> +
>> + /* register the tty driver */
>> + err = tty_register_driver(data->tty_drv);
>> + if (err) {
>> + pr_err("%s - tty_register_driver failed(%d)\n",
>> + __func__, err);
>> + put_tty_driver(data->tty_drv);
>> + goto err_rfkill;
>> + }
>> +
>> + /* minor (0) is now in use */
>> + w2sg_by_minor = data;
>> +
>> + tty_port_init(&data->port);
>> + data->port.ops = &w2sg_port_ops;
>> +
>> + data->dev = tty_port_register_device(&data->port,
>> + data->tty_drv, minor, &serdev->dev);
>
> Missing error handling.
Ok.
>
>> +
>> + /* keep off until user space requests the device */
>> + w2sg_set_power(data, false);
>> +
>> + return 0;
>> +
>> +err_rfkill:
>> + rfkill_destroy(rf_kill);
>> + serdev_device_close(data->uart);
>> +out:
>> + return err;
>> +}
>> +
>> +static void w2sg_remove(struct serdev_device *serdev)
>> +{
>> + struct w2sg_data *data = serdev_device_get_drvdata(serdev);
>> + int minor;
>> +
>> + cancel_delayed_work_sync(&data->work);
>> +
>> + /* what is the right sequence to avoid problems? */
>
> Clearly that's something you need to determine.
Well, I had hoped that the review process helps us here.
I do not have the same deep understanding of tty and serdev as all reviewers.
And there isn't much documentation beyond the code.
So in other words: I have no idea how to determine.
>
>> + serdev_device_close(data->uart);
>> +
>> + /* we should lookup in w2sg_by_minor */
>> + minor = 0;
>> + tty_unregister_device(data->tty_drv, minor);
>> +
>> + tty_unregister_driver(data->tty_drv);
>> +}
>> +
>> +static int __maybe_unused w2sg_suspend(struct device *dev)
>> +{
>> + struct w2sg_data *data = dev_get_drvdata(dev);
>> +
>> + data->suspended = true;
>> +
>> + cancel_delayed_work_sync(&data->work);
>> +
>> + w2sg_set_lna_power(data); /* shuts down if needed */
>> +
>> + if (data->state == W2SG_PULSE) {
>> + msleep(10);
>> + gpio_set_value_cansleep(data->on_off_gpio, 1);
>> + data->last_toggle = jiffies;
>> + data->is_on = !data->is_on;
>> + data->state = W2SG_NOPULSE;
>> + }
>> +
>> + if (data->state == W2SG_NOPULSE) {
>> + msleep(10);
>> + data->state = W2SG_IDLE;
>> + }
>> +
>> + if (data->is_on) {
>> + pr_info("GPS off for suspend %d %d %d\n", data->requested,
>> + data->is_on, data->lna_is_off);
>> +
>> + gpio_set_value_cansleep(data->on_off_gpio, 0);
>> + msleep(10);
>> + gpio_set_value_cansleep(data->on_off_gpio, 1);
>> + data->is_on = 0;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int __maybe_unused w2sg_resume(struct device *dev)
>> +{
>> + struct w2sg_data *data = dev_get_drvdata(dev);
>> +
>> + data->suspended = false;
>> +
>> + pr_info("GPS resuming %d %d %d\n", data->requested,
>> + data->is_on, data->lna_is_off);
>> +
>> + schedule_delayed_work(&data->work, 0); /* enables LNA if needed */
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id w2sg0004_of_match[] = {
>> + { .compatible = "wi2wi,w2sg0004" },
>> + { .compatible = "wi2wi,w2sg0084" },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, w2sg0004_of_match);
>> +
>> +SIMPLE_DEV_PM_OPS(w2sg_pm_ops, w2sg_suspend, w2sg_resume);
>> +
>> +static struct serdev_device_driver w2sg_driver = {
>> + .probe = w2sg_probe,
>> + .remove = w2sg_remove,
>> + .driver = {
>> + .name = "w2sg0004",
>> + .owner = THIS_MODULE,
>> + .pm = &w2sg_pm_ops,
>> + .of_match_table = of_match_ptr(w2sg0004_of_match)
>> + },
>> +};
>> +
>> +module_serdev_device_driver(w2sg_driver);
>> +
>> +MODULE_AUTHOR("NeilBrown <neilb@suse.de>");
>> +MODULE_AUTHOR("H. Nikolaus Schaller <hns@goldelico.com>");
>> +MODULE_DESCRIPTION("w2sg0004 GPS power management driver");
>> +MODULE_LICENSE("GPL v2");
>
> Johan
It looks as if you understand much better than us how this driver
should be written.
So, I would be happy if you take over development based on what we
suggest.
Please feel free to modify it as you think it is right. I can then
test your version on our hardware and report issues.
BR and thanks,
Nikolaus Schaller
^ permalink raw reply
* [PATCH v5 5/5] misc serdev: w2sg0004: add debugging code and Kconfig
From: H. Nikolaus Schaller @ 2017-12-22 14:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171222125150.GJ3374@localhost>
> Am 22.12.2017 um 13:51 schrieb Johan Hovold <johan@kernel.org>:
>
> On Fri, Dec 01, 2017 at 08:49:38AM +0100, H. Nikolaus Schaller wrote:
>> This allows to set CONFIG_W2SG0004_DEBUG which will
>> make the driver report more activities and it will turn on the
>> GPS module during boot while the driver assumes that it
>> is off. This can be used to debug the correct functioning of
>> the hardware. Therefore we add it as an option to the driver
>> because we think it is of general use (and a little tricky to
>> add by system testers).
>>
>> Normally it should be off.
>>
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> ---
>> drivers/misc/Kconfig | 8 ++++++++
>> drivers/misc/w2sg0004.c | 37 +++++++++++++++++++++++++++++++++++++
>> 2 files changed, 45 insertions(+)
>
> I'd say say this does not belong in the kernel at all.
There are other examples of such DEBUG options for drivers.
E.g. CONFIG_LIBERTAS_DEBUG.
It helps the hardware developer to test things and should of course
be disabled in a production kernel.
During hardware bringup I am always happy if someone else has shared
such tools instead of omitting them. So that we do not have to invent
them (again) and hack into our own kernel.
Of course, this should be discussed and you are open to take it
or leave it.
> And even if the
> power-state test were to be allowed, most of the pr_debugs would
> need to go.
I see.
"/* If you are writing a driver, please use dev_dbg instead */"
Well, nobody actively reads this comment in the header file...
Maybe checkpatch could be teached to warn?
> You really should be using dev_dbg and friends, which can
> already be enabled selectively at run time using dynamic debugging.
Well, in this case it is not needed to dynamically turn it on/off.
BR and thanks,
Nikolaus Schaller
^ permalink raw reply
* [PATCH v5 5/5] misc serdev: w2sg0004: add debugging code and Kconfig
From: Greg Kroah-Hartman @ 2017-12-22 14:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <A2720858-5172-4627-8133-C677E4B4DC72@goldelico.com>
On Fri, Dec 22, 2017 at 03:41:24PM +0100, H. Nikolaus Schaller wrote:
>
> > Am 22.12.2017 um 13:51 schrieb Johan Hovold <johan@kernel.org>:
> >
> > On Fri, Dec 01, 2017 at 08:49:38AM +0100, H. Nikolaus Schaller wrote:
> >> This allows to set CONFIG_W2SG0004_DEBUG which will
> >> make the driver report more activities and it will turn on the
> >> GPS module during boot while the driver assumes that it
> >> is off. This can be used to debug the correct functioning of
> >> the hardware. Therefore we add it as an option to the driver
> >> because we think it is of general use (and a little tricky to
> >> add by system testers).
> >>
> >> Normally it should be off.
> >>
> >> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> >> ---
> >> drivers/misc/Kconfig | 8 ++++++++
> >> drivers/misc/w2sg0004.c | 37 +++++++++++++++++++++++++++++++++++++
> >> 2 files changed, 45 insertions(+)
> >
> > I'd say say this does not belong in the kernel at all.
>
> There are other examples of such DEBUG options for drivers.
> E.g. CONFIG_LIBERTAS_DEBUG.
And that is wrong and should be removed.
> It helps the hardware developer to test things and should of course
> be disabled in a production kernel.
dev_dbg() should be used instead. And you can always hard-code #define DEBUG
at the top of your file when doing bringup.
thanks,
greg k-h
^ permalink raw reply
* [PATCH 06/10] arm64: handle 52-bit physical addresses in page table entries
From: Catalin Marinas @ 2017-12-22 15:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <a0a81fd9-e11c-bd85-5484-48b2e638c65a@arm.com>
On Mon, Dec 18, 2017 at 04:36:46PM +0000, Suzuki K. Poulose wrote:
> On 13/12/17 17:07, Kristina Martsenko wrote:
> > @@ -427,7 +451,7 @@ static inline phys_addr_t pmd_page_paddr(pmd_t pmd)
> > #define pte_set_fixmap_offset(pmd, addr) pte_set_fixmap(pte_offset_phys(pmd, addr))
> > #define pte_clear_fixmap() clear_fixmap(FIX_PTE)
> > -#define pmd_page(pmd) pfn_to_page(__phys_to_pfn(pmd_val(pmd) & PHYS_MASK))
> > +#define pmd_page(pmd) pfn_to_page(__phys_to_pfn(__pmd_to_phys(pmd)))
>
> You could simplify the above to :
>
> #define pmd_page(pmd) pfn_to_page(pmd_pfn(pmd))
>
> ? similarly for pud_page.
Just for the record as we already discussed: pmd_pfn(pmd) gives a
different result from __phys_to_pfn(__pmd_to_phys(pmd)). Weird but
pmd_pfn() is used with huge page stuff and it ensures that the pfn
corresponds to a PMD_SIZE-aligned page while pmd_page() is used to
retrieve the page of the pte pointed at by the pmd, so only PAGE_SIZE
aligned.
--
Catalin
^ permalink raw reply
* [PATCH 00/14] iio: triggers: add consumer support
From: Eugen Hristev @ 2017-12-22 15:07 UTC (permalink / raw)
To: linux-arm-kernel
Hello,
I would like to introduce his patch series which implements:
- new iio channel IIO_POSITION
- new inkern API to expose iio triggers to consumer drivers
- new bindings for trigger consumer drivers in device tree
- new input touchscreen driver for SAMA5D2
- implementation of POSITION and PRESSURE channels in sama5d2_adc driver
- devicetree changes for sama5d2 soc w.r.t. resistive touchscreen.
The SAMA5D2 SOC contains the ADC IP which has support
for a resistive touchscreen inside, using channels 0,1,2,3 from the ADC.
As discussed earlier on the mailing list, here is an implementation based
on the fact that the IIO device can expose the trigger and the required
channels to another driver (in our case the input touchscreen driver).
This touchscreen driver will consume the trigger and the values ( by
feeding them to the input subsystem).
Design decisions: The touchscreen driver will do a successful probe at
all times. Only on touch open and close, it will connect to the ADC device.
This is done to avoid a hard dependency between loading the ADC module and
the touchscreen one. So probe will always succeed and the dependency
is only soft.
Below it's an explanation of each patch in the series. Some changes were
required in the inkern API and the implementation was done in the ADC driver
and the touchscreen driver.
1: Added maintainers entry for new driver. This is done in commit:
MAINTAINERS: add sama5d2 resistive touchscreen
2: First we need a new set of channels named Position to report from the IIO
device the position on the touchpad. This is done in commit :
iio: Add channel for Position
3: we need device tree connection between the trigger producer and the
trigger consumer. I made new bindings very similar with the ones for the IIO
channels. This is done in the commit:
dt-bindings: iio: add binding support for iio trigger
provider/consumer
4: we need bindings for the input touchscreen driver, named sama5d2_rts
(resistive touch screen). This driver will be the consumer for the IIO
trigger and channels. This is done in the commit:
dt-bindings: input: touchscreen: sama5d2_rts: create bindings
5: we need some helper functions to get the trigger from the iio device
from inkern API. This is done in the commit:
iio: triggers: add helper function to retrieve trigger
6: we need helper functions to attach and detach a pollfunction to and
from the trigger itself. The trigger is registered by an IIO device, and
is still connected to it, but we need to be able to register a different
pollfunc from another driver. Thus I exposed this API to be inkern API.
This is done in commit:
iio: triggers: expose attach and detach helpers for pollfuncs
7: we need to be able to attach a pollfunc to a trigger, even if the
pollfunc is not coming from an iio device. Thus, register the pollfunc using
the iio_dev of the trigger (in case it's coming as NULL from the pollfunc).
This is done in the commit:
iio: triggers: on pollfunc attach, complete iio_dev if NULL
8: we need a private data on the pollfunc, such that the consumer
driver has a pointer to it's private data when the handler is called.
This is done in the commit:
iio: triggers: add private data to pollfuncs
9: we need to have inkern API to be able to look inside the devicetree
and find from the properties, which trigger(s) need to be connected.
Created some API that will look through the properties and return found
triggers. This is done in the commit:
iio: inkern: triggers: create helpers for OF trigger retrieval
10: in at91-sama5d2_adc we need to remove trigger on module remove in
order to not have stray triggers that are unusable after the module is
removed and reinserted. If such triggers stay registered, the code that
looks through the list of triggers will not work as intended.
This is done in commit:
iio: adc: at91-sama5d2_adc: force trigger removal on module remove
11: we need consecutive indexes for the channels provided by at91-sama5d2_adc
so that we optimize the index values and easier to connect the
touchscreen driver to the channels as a consumer. This is done in the commit:
iio: adc: at91-sama5d2_adc: optimize scan index for diff channels
12: the implementation of the channels and the support for position
conversion and pressure is done in the at91-sama5d2_adc driver.
The implementation provides a trigger named "touch" which can be
connected to a consumer driver.
Once a driver connects and attaches a pollfunc to this trigger, the
configure trigger callback is called, and then the ADC driver will
initialize pad measurement.
First step is to enable touchscreen 4wire support and enable
pen detect IRQ.
Once a pen is detected, a periodic trigger is setup to trigger every 2 ms
(e.g.) and sample the resistive touchscreen values. The trigger poll
is called, and the consumer driver is then woke up, and it can read the
respective channels for the values : X, and Y for position and pressure
channel.
Because only one trigger can be active in hardware in the same time,
while touching the pad, the ADC will block any attempt to use the
triggered buffer. Same, conversions using the software trigger are also
impossible (since the periodic trigger is setup).
If some driver wants to attach while the trigger is in use, it will also fail.
Once the pen is not detected anymore, the trigger is free for use (hardware
or software trigger, with or without DMA).
Channels 0,1,2 and 3 are unavailable if a touchscreen is enabled.
This is done in the commit:
iio: adc: at91-sama5d2_adc: support for position and pressure channels
13: the implementation of the basic touchscreen driver, which is an IIO
consumer.
The driver registers an input device and connects to the given IIO device from
devicetree. It requires an IIO trigger (acting as a consumer) and three IIO
channels : one for X position, one for Y position and one for pressure.
It the reports the values to the input subsystem.
This is done in the commit:
input: touchscreen: sama5d2_rts: SAMA5D2 Resistive touchscreen driver
14: Finally, we need to have the node in the sama5d2.dtsi,
as it's part of the SoC. This is done in the commit:
ARM: dts: at91: sama5d2: Add resistive touch device
This series is based on the discussion in mailing list and probably requires
some patching up, but hopefully the idea and the implementation of the inkern
trigger consumers and the splitting between the ADC driver and the input
driver is ok like this.
The implementation avoids using a MFD driver, which means to make three
drivers in fact, one for ADC, one for touch, with a shared IRQ, and a general
MFD driver that will be a placeholder for the two.
This implementation is based on patches initially written by
Swamy Bandaru Venkateshwara and Mohamed Jamsheeth Hajanajubudeen
Eugen Hristev (14):
MAINTAINERS: add sama5d2 resistive touchscreen
iio: Add channel for Position
dt-bindings: iio: add binding support for iio trigger
provider/consumer
dt-bindings: input: touchscreen: sama5d2_rts: create bindings
iio: triggers: add helper function to retrieve trigger
iio: triggers: expose attach and detach helpers for pollfuncs
iio: triggers: on pollfunc attach, complete iio_dev if NULL
iio: triggers: add private data to pollfuncs
iio: inkern: triggers: create helpers for OF trigger retrieval
iio: adc: at91-sama5d2_adc: force trigger removal on module remove
iio: adc: at91-sama5d2_adc: optimize scan index for diff channels
iio: adc: at91-sama5d2_adc: support for position and pressure channels
input: touchscreen: sama5d2_rts: SAMA5D2 Resistive touchscreen driver
ARM: dts: at91: sama5d2: Add resistive touch device
Documentation/ABI/testing/sysfs-bus-iio | 11 +
.../devicetree/bindings/iio/iio-bindings.txt | 52 ++-
.../bindings/input/touchscreen/sama5d2_rts.txt | 31 ++
MAINTAINERS | 6 +
arch/arm/boot/dts/sama5d2.dtsi | 12 +-
drivers/iio/adc/at91-sama5d2_adc.c | 460 ++++++++++++++++++++-
drivers/iio/iio_core_trigger.h | 21 +
drivers/iio/industrialio-core.c | 1 +
drivers/iio/industrialio-trigger.c | 42 +-
drivers/iio/inkern.c | 91 ++++
drivers/input/touchscreen/Kconfig | 13 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/sama5d2_rts.c | 287 +++++++++++++
include/linux/iio/trigger_consumer.h | 21 +
include/uapi/linux/iio/types.h | 1 +
tools/iio/iio_event_monitor.c | 2 +
16 files changed, 1036 insertions(+), 16 deletions(-)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/sama5d2_rts.txt
create mode 100644 drivers/input/touchscreen/sama5d2_rts.c
--
2.7.4
^ permalink raw reply
* [PATCH 01/14] MAINTAINERS: add sama5d2 resistive touchscreen
From: Eugen Hristev @ 2017-12-22 15:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1513955241-10985-1-git-send-email-eugen.hristev@microchip.com>
Add MAINTAINERS entry for sama5d2 resistive touchscreen driver
Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
MAINTAINERS | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 650aa0e..8beec28 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8957,6 +8957,12 @@ F: drivers/media/platform/atmel/atmel-isc.c
F: drivers/media/platform/atmel/atmel-isc-regs.h
F: devicetree/bindings/media/atmel-isc.txt
+MICROCHIP / ATMEL SAMA5D2 RESISTIVE TOUCHSCREEN DRIVER
+M: Eugen Hristev <eugen.hristev@microchip.com>
+L: linux-input at vger.kernel.org
+S: Maintained
+F: drivers/input/touchscreen/sama5d2_rts.c
+
MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER
M: Woojung Huh <Woojung.Huh@microchip.com>
M: Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
--
2.7.4
^ permalink raw reply related
* [PATCH 02/14] iio: Add channel for Position
From: Eugen Hristev @ 2017-12-22 15:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1513955241-10985-1-git-send-email-eugen.hristev@microchip.com>
Add new channel type for position on a pad.
These type of analog sensor represents the position of a pen
on a touchpad, and is represented as a voltage, which can be
converted to a position on X and Y axis on the pad.
The channel can then be consumed by a touchscreen driver or
read as-is for a raw indication of the touchpen on a touchpad.
Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
Documentation/ABI/testing/sysfs-bus-iio | 11 +++++++++++
drivers/iio/industrialio-core.c | 1 +
include/uapi/linux/iio/types.h | 1 +
tools/iio/iio_event_monitor.c | 2 ++
4 files changed, 15 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index a478740..d2b9e2f 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -190,6 +190,17 @@ Description:
but should match other such assignments on device).
Units after application of scale and offset are m/s^2.
+What: /sys/bus/iio/devices/iio:deviceX/in_position_x_raw
+What: /sys/bus/iio/devices/iio:deviceX/in_position_y_raw
+KernelVersion: 4.16
+Contact: linux-iio at vger.kernel.org
+Description:
+ Position in direction x or y on a pad (may be arbitrarily
+ assigned but should match other such assignments on device).
+ Units after application of scale and offset are millipercents
+ from the pad's size in both directions. Should be calibrated by
+ the consumer.
+
What: /sys/bus/iio/devices/iio:deviceX/in_anglvel_x_raw
What: /sys/bus/iio/devices/iio:deviceX/in_anglvel_y_raw
What: /sys/bus/iio/devices/iio:deviceX/in_anglvel_z_raw
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 2e8e36f..a4fa49b 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -85,6 +85,7 @@ static const char * const iio_chan_type_name_spec[] = {
[IIO_COUNT] = "count",
[IIO_INDEX] = "index",
[IIO_GRAVITY] = "gravity",
+ [IIO_POSITION] = "position",
};
static const char * const iio_modifier_names[] = {
diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
index 4213cdf..35e17da 100644
--- a/include/uapi/linux/iio/types.h
+++ b/include/uapi/linux/iio/types.h
@@ -44,6 +44,7 @@ enum iio_chan_type {
IIO_COUNT,
IIO_INDEX,
IIO_GRAVITY,
+ IIO_POSITION,
};
enum iio_modifier {
diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c
index b61245e..0c2b317 100644
--- a/tools/iio/iio_event_monitor.c
+++ b/tools/iio/iio_event_monitor.c
@@ -58,6 +58,7 @@ static const char * const iio_chan_type_name_spec[] = {
[IIO_PH] = "ph",
[IIO_UVINDEX] = "uvindex",
[IIO_GRAVITY] = "gravity",
+ [IIO_POSITION] = "position",
};
static const char * const iio_ev_type_text[] = {
@@ -151,6 +152,7 @@ static bool event_is_known(struct iio_event_data *event)
case IIO_PH:
case IIO_UVINDEX:
case IIO_GRAVITY:
+ case IIO_POSITION:
break;
default:
return false;
--
2.7.4
^ permalink raw reply related
* [PATCH 03/14] dt-bindings: iio: add binding support for iio trigger provider/consumer
From: Eugen Hristev @ 2017-12-22 15:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1513955241-10985-1-git-send-email-eugen.hristev@microchip.com>
Add bindings for producer/consumer for iio triggers.
Similar with iio channels, the iio triggers can be connected between drivers:
one driver will be a producer by registering iio triggers, and another driver
will connect as a consumer.
Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
.../devicetree/bindings/iio/iio-bindings.txt | 52 +++++++++++++++++++++-
1 file changed, 51 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
index 68d6f8c..d861f0df 100644
--- a/Documentation/devicetree/bindings/iio/iio-bindings.txt
+++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
@@ -11,6 +11,10 @@ value of a #io-channel-cells property in the IIO provider node.
[1] http://marc.info/?l=linux-iio&m=135902119507483&w=2
+Moreover, the provider can have a set of triggers that can be attached to
+from the consumer drivers.
+
+
==IIO providers==
Required properties:
@@ -18,6 +22,11 @@ Required properties:
with a single IIO output and 1 for nodes with multiple
IIO outputs.
+Optional properties:
+#io-trigger-cells: Number of cells for the IIO trigger specifier. Typically 0
+ for nodes with a single IIO trigger and 1 for nodes with
+ multiple IIO triggers.
+
Example for a simple configuration with no trigger:
adc: voltage-sensor at 35 {
@@ -26,7 +35,7 @@ Example for a simple configuration with no trigger:
#io-channel-cells = <1>;
};
-Example for a configuration with trigger:
+Example for a configuration with channels provided by trigger:
adc at 35 {
compatible = "some-vendor,some-adc";
@@ -42,6 +51,17 @@ Example for a configuration with trigger:
};
};
+Example for a configuration for a trigger provider:
+
+ adc: sensor-with-trigger at 35 {
+ compatible = "some-vendor,some-adc";
+ reg = <0x35>;
+ #io-channel-cells = <1>;
+ #io-trigger-cells = <1>;
+ /* other properties */
+ };
+
+
==IIO consumers==
Required properties:
@@ -61,16 +81,38 @@ io-channel-ranges:
IIO channels from this node. Useful for bus nodes to provide
and IIO channel to their children.
+io-triggers: List of phandle and IIO specifier pairs, one pair
+ for each trigger input to the device. Note: if the
+ IIO trigger provider specifies '0' for #io-trigger-cells,
+ then only the phandle portion of the pair will appear.
+
+io-trigger-names:
+ List of IIO trigger input name strings sorted in the same
+ order as the io-triggers property. Consumers drivers
+ will use io-trigger-names to match IIO trigger input names
+ with IIO specifiers.
+
+io-trigger-ranges:
+ Empty property indicating that child nodes can inherit named
+ IIO triggers from this node. Useful for bus nodes to provide
+ IIO triggers to their children.
+
For example:
device {
io-channels = <&adc 1>, <&ref 0>;
io-channel-names = "vcc", "vdd";
+ io-triggers = <&adc 0>, <&adc 1>;
+ io-trigger-names = "continuous", "external";
};
This represents a device with two IIO inputs, named "vcc" and "vdd".
The vcc channel is connected to output 1 of the &adc device, and the
vdd channel is connected to output 0 of the &ref device.
+The device also connects to two IIO trigger inputs, named "continuous" and
+"external". The continuous trigger is connected to the trigger output 0 of the
+&adc device, and the external trigger is connected to the trigger output 1 of
+the same &adc device.
==Example==
@@ -78,6 +120,7 @@ vdd channel is connected to output 0 of the &ref device.
compatible = "maxim,max1139";
reg = <0x35>;
#io-channel-cells = <1>;
+ #io-trigger-cells = <1>;
};
...
@@ -94,4 +137,11 @@ vdd channel is connected to output 0 of the &ref device.
compatible = "some-consumer";
io-channels = <&adc 10>, <&adc 11>;
io-channel-names = "adc1", "adc2";
+ io-triggers = <&adc 0>;
+ };
+
+ some_other_consumer {
+ compatible = "some-other-consumer";
+ io-triggers = <&adc 1>, <&adc 2>;
+ io-trigger-names = "edge-triggered", "pwm-generated";
};
--
2.7.4
^ permalink raw reply related
* [PATCH 04/14] dt-bindings: input: touchscreen: sama5d2_rts: create bindings
From: Eugen Hristev @ 2017-12-22 15:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1513955241-10985-1-git-send-email-eugen.hristev@microchip.com>
Added bindings for Microchip SAMA5D2 resistive touchscreen.
Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
.../bindings/input/touchscreen/sama5d2_rts.txt | 31 ++++++++++++++++++++++
1 file changed, 31 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/sama5d2_rts.txt
diff --git a/Documentation/devicetree/bindings/input/touchscreen/sama5d2_rts.txt b/Documentation/devicetree/bindings/input/touchscreen/sama5d2_rts.txt
new file mode 100644
index 0000000..ea52a5c
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/sama5d2_rts.txt
@@ -0,0 +1,31 @@
+Microchip SAMA5D2 resistive touchscreen
+
+Required properties:
+
+ - compatible: microchip,sama5d2-resistive-touch
+
+Optional properties:
+
+The device must be connected to an IIO device that provides channels for
+position and pressure measurement, and a trigger output that will periodically
+trigger when the touch is pressed.
+Refer to ../iio/iio-bindings.txt for details
+
+ - io-channels: can have two or three channels connected to an IIO device.
+These should correspond to the channels exposed by the IIO device and should
+have the right index as the ADC registers them.
+ - io-channel-names: must have the two or three channels' names :
+"x", "y", or "x", "y" and "pressure"
+ - io-triggers: can have a trigger input connected to an IIO device.
+ - microchip,pressure-threshold: a pressure threshold for the touchscreen.
+ Represented by an integer value.
+
+Example:
+
+ resistive_touch: resistive-touch {
+ compatible = "microchip,sama5d2-resistive-touch";
+ microchip,pressure-threshold = <3000>;
+ io-channels = <&adc 19>, <&adc 20>, <&adc 21>;
+ io-channel-names = "x", "y", "pressure";
+ io-triggers = <&adc 1>;
+ };
--
2.7.4
^ permalink raw reply related
* [PATCH 05/14] iio: triggers: add helper function to retrieve trigger
From: Eugen Hristev @ 2017-12-22 15:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1513955241-10985-1-git-send-email-eugen.hristev@microchip.com>
Add a helper function that will retrieve a trigger by index
from a device. This is intended to be used in trigger consumer drivers.
Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
drivers/iio/iio_core_trigger.h | 21 +++++++++++++++++++++
drivers/iio/industrialio-trigger.c | 23 +++++++++++++++++++++++
2 files changed, 44 insertions(+)
diff --git a/drivers/iio/iio_core_trigger.h b/drivers/iio/iio_core_trigger.h
index 1fdb1e4..14c4253 100644
--- a/drivers/iio/iio_core_trigger.h
+++ b/drivers/iio/iio_core_trigger.h
@@ -21,6 +21,15 @@ void iio_device_register_trigger_consumer(struct iio_dev *indio_dev);
**/
void iio_device_unregister_trigger_consumer(struct iio_dev *indio_dev);
+/**
+ * iio_trigger_find_from_device() - get the trigger from the device having the
+ * index given.
+ * @indio_dev: iio_dev where to get the trigger from
+ * @index: the index of the trigger to retrieve
+ **/
+__maybe_unused struct iio_trigger *
+iio_trigger_find_from_device(struct iio_dev *indio_dev, u32 index);
+
#else
/**
@@ -40,4 +49,16 @@ static void iio_device_unregister_trigger_consumer(struct iio_dev *indio_dev)
{
}
+/**
+ * iio_trigger_find_from_device() - get the trigger from the device having the
+ * index given.
+ * @indio_dev: iio_dev where to get the trigger from
+ * @index: the index of the trigger to retrieve
+ **/
+static __maybe_unused struct iio_trigger *
+iio_trigger_find_from_device(struct iio_dev *indio_dev, u32 index)
+{
+ return NULL;
+}
+
#endif /* CONFIG_TRIGGER_CONSUMER */
diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
index ce66699..cbaa079 100644
--- a/drivers/iio/industrialio-trigger.c
+++ b/drivers/iio/industrialio-trigger.c
@@ -134,6 +134,29 @@ int iio_trigger_set_immutable(struct iio_dev *indio_dev, struct iio_trigger *tri
}
EXPORT_SYMBOL(iio_trigger_set_immutable);
+/* Find the trigger from the given device corresponding to given index */
+struct __maybe_unused iio_trigger *
+iio_trigger_find_from_device(struct iio_dev *indio_dev, u32 index)
+{
+ struct iio_trigger *iter, *trig = NULL;
+ u32 i = 0;
+
+ mutex_lock(&iio_trigger_list_lock);
+
+ list_for_each_entry(iter, &iio_trigger_list, list) {
+ if (!iio_trigger_validate_own_device(iter, indio_dev)) {
+ if (i == index) {
+ trig = iter;
+ break;
+ }
+ i++;
+ }
+ }
+ mutex_unlock(&iio_trigger_list_lock);
+
+ return trig;
+}
+
/* Search for trigger by name, assuming iio_trigger_list_lock held */
static struct iio_trigger *__iio_trigger_find_by_name(const char *name)
{
--
2.7.4
^ permalink raw reply related
* [PATCH 06/14] iio: triggers: expose attach and detach helpers for pollfuncs
From: Eugen Hristev @ 2017-12-22 15:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1513955241-10985-1-git-send-email-eugen.hristev@microchip.com>
Make attach and detach functions for pollfuncs available as symbols.
These functions are required in the trigger consumer drivers in order
to register their own pollfunctions to iio devices.
Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
drivers/iio/industrialio-trigger.c | 10 ++++++----
include/linux/iio/trigger_consumer.h | 9 +++++++++
2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
index cbaa079..8565c92 100644
--- a/drivers/iio/industrialio-trigger.c
+++ b/drivers/iio/industrialio-trigger.c
@@ -265,8 +265,8 @@ static void iio_trigger_put_irq(struct iio_trigger *trig, int irq)
* the relevant function is in there may be the best option.
*/
/* Worth protecting against double additions? */
-static int iio_trigger_attach_poll_func(struct iio_trigger *trig,
- struct iio_poll_func *pf)
+int iio_trigger_attach_poll_func(struct iio_trigger *trig,
+ struct iio_poll_func *pf)
{
int ret = 0;
bool notinuse
@@ -312,9 +312,10 @@ static int iio_trigger_attach_poll_func(struct iio_trigger *trig,
module_put(pf->indio_dev->driver_module);
return ret;
}
+EXPORT_SYMBOL(iio_trigger_attach_poll_func);
-static int iio_trigger_detach_poll_func(struct iio_trigger *trig,
- struct iio_poll_func *pf)
+int iio_trigger_detach_poll_func(struct iio_trigger *trig,
+ struct iio_poll_func *pf)
{
int ret = 0;
bool no_other_users
@@ -334,6 +335,7 @@ static int iio_trigger_detach_poll_func(struct iio_trigger *trig,
return ret;
}
+EXPORT_SYMBOL(iio_trigger_detach_poll_func);
irqreturn_t iio_pollfunc_store_time(int irq, void *p)
{
diff --git a/include/linux/iio/trigger_consumer.h b/include/linux/iio/trigger_consumer.h
index c4f8c74..aeefcdb 100644
--- a/include/linux/iio/trigger_consumer.h
+++ b/include/linux/iio/trigger_consumer.h
@@ -60,4 +60,13 @@ void iio_trigger_notify_done(struct iio_trigger *trig);
int iio_triggered_buffer_postenable(struct iio_dev *indio_dev);
int iio_triggered_buffer_predisable(struct iio_dev *indio_dev);
+/*
+ * Two functions for the uncommon case when we need to attach or detach
+ * a specific pollfunc to and from a trigger
+ */
+int iio_trigger_attach_poll_func(struct iio_trigger *trig,
+ struct iio_poll_func *pf);
+
+int iio_trigger_detach_poll_func(struct iio_trigger *trig,
+ struct iio_poll_func *pf);
#endif
--
2.7.4
^ permalink raw reply related
* [PATCH 07/14] iio: triggers: on pollfunc attach, complete iio_dev if NULL
From: Eugen Hristev @ 2017-12-22 15:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1513955241-10985-1-git-send-email-eugen.hristev@microchip.com>
When attaching a pollfunc to a trigger, if the pollfunc does not
have an associated iio_dev pointer, just use the private data
iio_dev pointer from the trigger to fill in the poll func required
iio_dev reference.
Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
drivers/iio/industrialio-trigger.c | 9 +++++++++
include/linux/iio/trigger_consumer.h | 2 ++
2 files changed, 11 insertions(+)
diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
index 8565c92..ab180bd 100644
--- a/drivers/iio/industrialio-trigger.c
+++ b/drivers/iio/industrialio-trigger.c
@@ -272,6 +272,15 @@ int iio_trigger_attach_poll_func(struct iio_trigger *trig,
bool notinuse
= bitmap_empty(trig->pool, CONFIG_IIO_CONSUMERS_PER_TRIGGER);
+ /*
+ * If we did not get a iio_dev in the poll func, attempt to
+ * obtain the trigger's owner's device struct
+ */
+ if (!pf->indio_dev)
+ pf->indio_dev = iio_trigger_get_drvdata(trig);
+ if (!pf->indio_dev)
+ return -EINVAL;
+
/* Prevent the module from being removed whilst attached to a trigger */
__module_get(pf->indio_dev->driver_module);
diff --git a/include/linux/iio/trigger_consumer.h b/include/linux/iio/trigger_consumer.h
index aeefcdb..36e2a02 100644
--- a/include/linux/iio/trigger_consumer.h
+++ b/include/linux/iio/trigger_consumer.h
@@ -63,6 +63,8 @@ int iio_triggered_buffer_predisable(struct iio_dev *indio_dev);
/*
* Two functions for the uncommon case when we need to attach or detach
* a specific pollfunc to and from a trigger
+ * If the pollfunc has a NULL iio_dev pointer, it will be filled from the
+ * trigger struct.
*/
int iio_trigger_attach_poll_func(struct iio_trigger *trig,
struct iio_poll_func *pf);
--
2.7.4
^ permalink raw reply related
* [PATCH 08/14] iio: triggers: add private data to pollfuncs
From: Eugen Hristev @ 2017-12-22 15:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1513955241-10985-1-git-send-email-eugen.hristev@microchip.com>
Add a private data pointer field to pollfunc struct.
This is useful in the trigger handler to get specific data
for the driver registering the pollfunc.
Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
include/linux/iio/trigger_consumer.h | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/include/linux/iio/trigger_consumer.h b/include/linux/iio/trigger_consumer.h
index 36e2a02..13be595 100644
--- a/include/linux/iio/trigger_consumer.h
+++ b/include/linux/iio/trigger_consumer.h
@@ -29,6 +29,7 @@ struct iio_trigger;
* @timestamp: some devices need a timestamp grabbed as soon
* as possible after the trigger - hence handler
* passes it via here.
+ * @p: private data for the poll func owner.
**/
struct iio_poll_func {
struct iio_dev *indio_dev;
@@ -38,6 +39,7 @@ struct iio_poll_func {
char *name;
int irq;
s64 timestamp;
+ void *p;
};
@@ -49,6 +51,13 @@ struct iio_poll_func
const char *fmt,
...);
void iio_dealloc_pollfunc(struct iio_poll_func *pf);
+
+static inline void
+iio_pollfunc_set_private_data(struct iio_poll_func *pf, void *p)
+{
+ pf->p = p;
+}
+
irqreturn_t iio_pollfunc_store_time(int irq, void *p);
void iio_trigger_notify_done(struct iio_trigger *trig);
--
2.7.4
^ permalink raw reply related
* [PATCH 09/14] iio: inkern: triggers: create helpers for OF trigger retrieval
From: Eugen Hristev @ 2017-12-22 15:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1513955241-10985-1-git-send-email-eugen.hristev@microchip.com>
Create helper API to get trigger information from OF regarding
trigger producer/consumer for iio triggers.
The functions will search for matching trigger by name, similar
with channel retrieval
Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
drivers/iio/inkern.c | 91 ++++++++++++++++++++++++++++++++++++
include/linux/iio/trigger_consumer.h | 1 +
2 files changed, 92 insertions(+)
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index 069defc..58bd18d 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -14,9 +14,13 @@
#include <linux/iio/iio.h>
#include "iio_core.h"
+#include "iio_core_trigger.h"
#include <linux/iio/machine.h>
#include <linux/iio/driver.h>
#include <linux/iio/consumer.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+
struct iio_map_internal {
struct iio_dev *indio_dev;
@@ -262,6 +266,39 @@ static struct iio_channel *of_iio_channel_get_all(struct device *dev)
return ERR_PTR(ret);
}
+#ifdef CONFIG_IIO_TRIGGER
+
+static struct iio_trigger *of_iio_trigger_get(struct device_node *np, int index)
+{
+ struct device *idev;
+ struct iio_dev *indio_dev;
+ int err;
+ struct of_phandle_args iiospec;
+ struct iio_trigger *trig;
+
+ err = of_parse_phandle_with_args(np, "io-triggers",
+ "#io-trigger-cells",
+ index, &iiospec);
+ if (err)
+ return ERR_PTR(err);
+
+ idev = bus_find_device(&iio_bus_type, NULL, iiospec.np,
+ iio_dev_node_match);
+ of_node_put(iiospec.np);
+ if (!idev)
+ return ERR_PTR(-EPROBE_DEFER);
+
+ indio_dev = dev_to_iio_dev(idev);
+
+ trig = iio_trigger_find_from_device(indio_dev, iiospec.args[0]);
+
+ if (!trig)
+ return ERR_PTR(-ENODEV);
+
+ return trig;
+}
+#endif /* CONFIG_IIO_TRIGGER */
+
#else /* CONFIG_OF */
static inline struct iio_channel *
@@ -275,6 +312,12 @@ static inline struct iio_channel *of_iio_channel_get_all(struct device *dev)
return NULL;
}
+static inline struct iio_trigger *of_iio_trigger_get(struct device_node *np,
+ int index)
+{
+ return NULL;
+}
+
#endif /* CONFIG_OF */
static struct iio_channel *iio_channel_get_sys(const char *name,
@@ -927,3 +970,51 @@ ssize_t iio_write_channel_ext_info(struct iio_channel *chan, const char *attr,
chan->channel, buf, len);
}
EXPORT_SYMBOL_GPL(iio_write_channel_ext_info);
+
+#ifdef CONFIG_IIO_TRIGGER
+struct iio_trigger *iio_trigger_find(struct device *dev, char *name)
+{
+ struct device_node *np = dev->of_node;
+ struct iio_trigger *trig = NULL;
+
+ /* Walk up the tree of devices looking for a matching iio trigger */
+ while (np) {
+ int index = 0;
+
+ /*
+ * For named iio triggers, first look up the name in the
+ * "io-trigger-names" property. If it cannot be found, the
+ * index will be an error code, and of_iio_trigger_get()
+ * will fail.
+ */
+ if (name)
+ index = of_property_match_string(np, "io-trigger-names",
+ name);
+ trig = of_iio_trigger_get(np, index);
+ if (!IS_ERR(trig) || PTR_ERR(trig) == -EPROBE_DEFER) {
+ break;
+ } else if (name && index >= 0) {
+ pr_err("ERROR: could not get IIO trigger %pOF:%s(%i)\n",
+ np, name ? name : "", index);
+ return trig;
+ }
+
+ /*
+ * No matching IIO trigger found on this node.
+ * If the parent node has a "io-trigger-ranges" property,
+ * then we can try one of its channels.
+ */
+ np = np->parent;
+ if (np && !of_get_property(np, "io-trigger-ranges", NULL))
+ return trig;
+ }
+
+ if (!trig || (IS_ERR(trig) && PTR_ERR(trig) != -EPROBE_DEFER))
+ dev_dbg(dev, "error retrieving trigger information\n");
+ else
+ dev_dbg(dev, "trigger found: %s\n", name);
+
+ return trig;
+}
+EXPORT_SYMBOL_GPL(iio_trigger_find);
+#endif /* CONFIG_IIO_TRIGGER */
diff --git a/include/linux/iio/trigger_consumer.h b/include/linux/iio/trigger_consumer.h
index 13be595..b2fc485 100644
--- a/include/linux/iio/trigger_consumer.h
+++ b/include/linux/iio/trigger_consumer.h
@@ -62,6 +62,7 @@ irqreturn_t iio_pollfunc_store_time(int irq, void *p);
void iio_trigger_notify_done(struct iio_trigger *trig);
+struct iio_trigger *iio_trigger_find(struct device *dev, char *name);
/*
* Two functions for common case where all that happens is a pollfunc
* is attached and detached from a trigger
--
2.7.4
^ permalink raw reply related
* [PATCH 10/14] iio: adc: at91-sama5d2_adc: force trigger removal on module remove
From: Eugen Hristev @ 2017-12-22 15:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1513955241-10985-1-git-send-email-eugen.hristev@microchip.com>
On module remove, if we do not call trigger remove, the trigger
stays in the subsystem, and on further module insert, we will have
multiple triggers, and the old one is not usable.
Have to call the remove function on module remove to solve this.
Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
drivers/iio/adc/at91-sama5d2_adc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
index 4eff835..7b9febc 100644
--- a/drivers/iio/adc/at91-sama5d2_adc.c
+++ b/drivers/iio/adc/at91-sama5d2_adc.c
@@ -1180,6 +1180,9 @@ static int at91_adc_remove(struct platform_device *pdev)
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
struct at91_adc_state *st = iio_priv(indio_dev);
+ if (st->selected_trig->hw_trig)
+ devm_iio_trigger_unregister(&indio_dev->dev, st->trig);
+
iio_device_unregister(indio_dev);
at91_adc_dma_disable(pdev);
--
2.7.4
^ permalink raw reply related
* [PATCH 11/14] iio: adc: at91-sama5d2_adc: optimize scan index for diff channels
From: Eugen Hristev @ 2017-12-22 15:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1513955241-10985-1-git-send-email-eugen.hristev@microchip.com>
Optimize the scan index for the differential channels. Before, it
was single channel count + index of the first single channel
number of the differential pair. (e.g. 11+0, +2, +4, etc.)
Divide that number by two (since it's always even), and add it up
as a scan index to have consecutive numbered channels in the
index.
Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
drivers/iio/adc/at91-sama5d2_adc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
index 7b9febc..9610393 100644
--- a/drivers/iio/adc/at91-sama5d2_adc.c
+++ b/drivers/iio/adc/at91-sama5d2_adc.c
@@ -209,7 +209,7 @@
.channel = num, \
.channel2 = num2, \
.address = addr, \
- .scan_index = num + AT91_SAMA5D2_SINGLE_CHAN_CNT, \
+ .scan_index = (num >> 1) + AT91_SAMA5D2_SINGLE_CHAN_CNT,\
.scan_type = { \
.sign = 's', \
.realbits = 12, \
--
2.7.4
^ permalink raw reply related
* [PATCH 12/14] iio: adc: at91-sama5d2_adc: support for position and pressure channels
From: Eugen Hristev @ 2017-12-22 15:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1513955241-10985-1-git-send-email-eugen.hristev@microchip.com>
The ADC IP supports position and pressure measurements for a touchpad
connected on channels 0,1,2,3 for a 4-wire touchscreen with pressure
measurement support.
Using the inkern API, a driver can request a trigger and read the
channel values from the ADC.
The implementation provides a trigger named "touch" which can be
connected to a consumer driver.
Once a driver connects and attaches a pollfunc to this trigger, the
configure trigger callback is called, and then the ADC driver will
initialize pad measurement.
First step is to enable touchscreen 4wire support and enable
pen detect IRQ.
Once a pen is detected, a periodic trigger is setup to trigger every
2 ms (e.g.) and sample the resistive touchscreen values. The trigger poll
is called, and the consumer driver is then woke up, and it can read the
respective channels for the values : X, and Y for position and pressure
channel.
Because only one trigger can be active in hardware in the same time,
while touching the pad, the ADC will block any attempt to use the
triggered buffer. Same, conversions using the software trigger are also
impossible (since the periodic trigger is setup).
If some driver wants to attach while the trigger is in use, it will
also fail.
Once the pen is not detected anymore, the trigger is free for use (hardware
or software trigger, with or without DMA).
Channels 0,1,2 and 3 are unavailable if a touchscreen is enabled.
Some parts of this patch are based on initial original work by
Mohamed Jamsheeth Hajanajubudeen and Bandaru Venkateswara Swamy
Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
drivers/iio/adc/at91-sama5d2_adc.c | 455 ++++++++++++++++++++++++++++++++++++-
1 file changed, 446 insertions(+), 9 deletions(-)
diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
index 9610393..79eb197 100644
--- a/drivers/iio/adc/at91-sama5d2_adc.c
+++ b/drivers/iio/adc/at91-sama5d2_adc.c
@@ -102,14 +102,26 @@
#define AT91_SAMA5D2_LCDR 0x20
/* Interrupt Enable Register */
#define AT91_SAMA5D2_IER 0x24
+/* Interrupt Enable Register - TS X measurement ready */
+#define AT91_SAMA5D2_IER_XRDY BIT(20)
+/* Interrupt Enable Register - TS Y measurement ready */
+#define AT91_SAMA5D2_IER_YRDY BIT(21)
+/* Interrupt Enable Register - TS pressure measurement ready */
+#define AT91_SAMA5D2_IER_PRDY BIT(22)
/* Interrupt Enable Register - general overrun error */
#define AT91_SAMA5D2_IER_GOVRE BIT(25)
+/* Interrupt Enable Register - Pen detect */
+#define AT91_SAMA5D2_IER_PEN BIT(29)
+/* Interrupt Enable Register - No pen detect */
+#define AT91_SAMA5D2_IER_NOPEN BIT(30)
/* Interrupt Disable Register */
#define AT91_SAMA5D2_IDR 0x28
/* Interrupt Mask Register */
#define AT91_SAMA5D2_IMR 0x2c
/* Interrupt Status Register */
#define AT91_SAMA5D2_ISR 0x30
+/* Interrupt Status Register - Pen touching sense status */
+#define AT91_SAMA5D2_ISR_PENS BIT(31)
/* Last Channel Trigger Mode Register */
#define AT91_SAMA5D2_LCTMR 0x34
/* Last Channel Compare Window Register */
@@ -131,8 +143,37 @@
#define AT91_SAMA5D2_CDR0 0x50
/* Analog Control Register */
#define AT91_SAMA5D2_ACR 0x94
+/* Analog Control Register - Pen detect sensitivity mask */
+#define AT91_SAMA5D2_ACR_PENDETSENS_MASK GENMASK(0, 1)
/* Touchscreen Mode Register */
#define AT91_SAMA5D2_TSMR 0xb0
+/* Touchscreen Mode Register - No touch mode */
+#define AT91_SAMA5D2_TSMR_TSMODE_NONE 0
+/* Touchscreen Mode Register - 4 wire screen, no pressure measurement */
+#define AT91_SAMA5D2_TSMR_TSMODE_4WIRE_NO_PRESS 1
+/* Touchscreen Mode Register - 4 wire screen, pressure measurement */
+#define AT91_SAMA5D2_TSMR_TSMODE_4WIRE_PRESS 2
+/* Touchscreen Mode Register - 5 wire screen */
+#define AT91_SAMA5D2_TSMR_TSMODE_5WIRE 3
+/* Touchscreen Mode Register - Average samples mask */
+#define AT91_SAMA5D2_TSMR_TSAV_MASK (3 << 4)
+/* Touchscreen Mode Register - Average samples */
+#define AT91_SAMA5D2_TSMR_TSAV(x) ((x) << 4)
+/* Touchscreen Mode Register - Touch/trigger frequency ratio mask */
+#define AT91_SAMA5D2_TSMR_TSFREQ_MASK (0xf << 8)
+/* Touchscreen Mode Register - Touch/trigger freqency ratio */
+#define AT91_SAMA5D2_TSMR_TSFREQ(x) ((x) << 8)
+/* Touchscreen Mode Register - Pen Debounce Time mask */
+#define AT91_SAMA5D2_TSMR_PENDBC_MASK (0xf << 28)
+/* Touchscreen Mode Register - Pen Debounce Time */
+#define AT91_SAMA5D2_TSMR_PENDBC(x) ((x) << 28)
+/* Touchscreen Mode Register - No DMA for touch measurements */
+#define AT91_SAMA5D2_TSMR_NOTSDMA BIT(22)
+/* Touchscreen Mode Register - Disable pen detection */
+#define AT91_SAMA5D2_TSMR_PENDET_DIS (0 << 24)
+/* Touchscreen Mode Register - Enable pen detection */
+#define AT91_SAMA5D2_TSMR_PENDET_ENA BIT(24)
+
/* Touchscreen X Position Register */
#define AT91_SAMA5D2_XPOSR 0xb4
/* Touchscreen Y Position Register */
@@ -151,7 +192,12 @@
#define AT91_SAMA5D2_TRGR_TRGMOD_EXT_TRIG_FALL 2
/* Trigger Mode external trigger any edge */
#define AT91_SAMA5D2_TRGR_TRGMOD_EXT_TRIG_ANY 3
-
+/* Trigger Mode internal periodic */
+#define AT91_SAMA5D2_TRGR_TRGMOD_PERIODIC 5
+/* Trigger Mode - trigger period mask */
+#define AT91_SAMA5D2_TRGR_TRGPER_MASK (0xffff << 16)
+/* Trigger Mode - trigger period */
+#define AT91_SAMA5D2_TRGR_TRGPER(x) ((x) << 16)
/* Correction Select Register */
#define AT91_SAMA5D2_COSR 0xd0
/* Correction Value Register */
@@ -169,6 +215,21 @@
#define AT91_SAMA5D2_SINGLE_CHAN_CNT 12
#define AT91_SAMA5D2_DIFF_CHAN_CNT 6
+#define AT91_SAMA5D2_TIMESTAMP_CHAN_IDX (AT91_SAMA5D2_SINGLE_CHAN_CNT + \
+ AT91_SAMA5D2_DIFF_CHAN_CNT + 1)
+
+#define AT91_SAMA5D2_TOUCH_X_CHAN_IDX (AT91_SAMA5D2_TIMESTAMP_CHAN_IDX + 1)
+#define AT91_SAMA5D2_TOUCH_Y_CHAN_IDX (AT91_SAMA5D2_TOUCH_X_CHAN_IDX + 1)
+#define AT91_SAMA5D2_TOUCH_P_CHAN_IDX (AT91_SAMA5D2_TOUCH_Y_CHAN_IDX + 1)
+
+#define TOUCH_SAMPLE_PERIOD_US 2000 /* 2ms */
+#define TOUCH_PEN_DETECT_DEBOUNCE_US 200
+
+#define XYZ_MASK GENMASK(11, 0)
+
+#define MAX_POS_BITS 12
+
+#define AT91_ADC_TOUCH_TRIG_SHORTNAME "touch"
/*
* Maximum number of bytes to hold conversion from all channels
* without the timestamp.
@@ -222,6 +283,37 @@
.indexed = 1, \
}
+#define AT91_SAMA5D2_CHAN_TOUCH(num, name, mod) \
+ { \
+ .type = IIO_POSITION, \
+ .modified = 1, \
+ .channel = num, \
+ .channel2 = mod, \
+ .scan_index = num, \
+ .scan_type = { \
+ .sign = 'u', \
+ .realbits = 12, \
+ .storagebits = 16, \
+ }, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
+ .datasheet_name = name, \
+ }
+#define AT91_SAMA5D2_CHAN_PRESSURE(num, name) \
+ { \
+ .type = IIO_PRESSURE, \
+ .channel = num, \
+ .scan_index = num, \
+ .scan_type = { \
+ .sign = 'u', \
+ .realbits = 12, \
+ .storagebits = 16, \
+ }, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
+ .datasheet_name = name, \
+ }
+
#define at91_adc_readl(st, reg) readl_relaxed(st->base + reg)
#define at91_adc_writel(st, reg, val) writel_relaxed(val, st->base + reg)
@@ -239,6 +331,20 @@ struct at91_adc_trigger {
};
/**
+ * at91_adc_touch - at91-sama5d2 touchscreen information struct
+ * @trig: hold the start timestamp of dma operation
+ * @sample_period_val: the value for periodic trigger interval
+ * @touching: is the pen touching the screen or not
+ * @x_pos: temporary placeholder for pressure computation
+ */
+struct at91_adc_touch {
+ struct iio_trigger *trig;
+ u16 sample_period_val;
+ bool touching;
+ u32 x_pos;
+};
+
+/**
* at91_adc_dma - at91-sama5d2 dma information struct
* @dma_chan: the dma channel acquired
* @rx_buf: dma coherent allocated area
@@ -267,18 +373,22 @@ struct at91_adc_state {
struct regulator *reg;
struct regulator *vref;
int vref_uv;
+ unsigned int current_sample_rate;
struct iio_trigger *trig;
const struct at91_adc_trigger *selected_trig;
const struct iio_chan_spec *chan;
bool conversion_done;
u32 conversion_value;
+ bool touch_requested;
struct at91_adc_soc_info soc_info;
wait_queue_head_t wq_data_available;
struct at91_adc_dma dma_st;
+ struct at91_adc_touch touch_st;
u16 buffer[AT91_BUFFER_MAX_HWORDS];
/*
* lock to prevent concurrent 'single conversion' requests through
- * sysfs.
+ * sysfs. Also protects when enabling or disabling touchscreen
+ * producer mode and checking if this mode is enabled or not.
*/
struct mutex lock;
};
@@ -310,6 +420,7 @@ static const struct at91_adc_trigger at91_adc_trigger_list[] = {
},
};
+/* channel order is not subject to change. inkern consumers rely on this */
static const struct iio_chan_spec at91_adc_channels[] = {
AT91_SAMA5D2_CHAN_SINGLE(0, 0x50),
AT91_SAMA5D2_CHAN_SINGLE(1, 0x54),
@@ -329,10 +440,103 @@ static const struct iio_chan_spec at91_adc_channels[] = {
AT91_SAMA5D2_CHAN_DIFF(6, 7, 0x68),
AT91_SAMA5D2_CHAN_DIFF(8, 9, 0x70),
AT91_SAMA5D2_CHAN_DIFF(10, 11, 0x78),
- IIO_CHAN_SOFT_TIMESTAMP(AT91_SAMA5D2_SINGLE_CHAN_CNT
- + AT91_SAMA5D2_DIFF_CHAN_CNT + 1),
+ IIO_CHAN_SOFT_TIMESTAMP(AT91_SAMA5D2_TIMESTAMP_CHAN_IDX),
+ AT91_SAMA5D2_CHAN_TOUCH(AT91_SAMA5D2_TOUCH_X_CHAN_IDX, "x", IIO_MOD_X),
+ AT91_SAMA5D2_CHAN_TOUCH(AT91_SAMA5D2_TOUCH_Y_CHAN_IDX, "y", IIO_MOD_Y),
+ AT91_SAMA5D2_CHAN_PRESSURE(AT91_SAMA5D2_TOUCH_P_CHAN_IDX, "pressure"),
};
+static int at91_adc_configure_touch(struct at91_adc_state *st, bool state)
+{
+ u32 clk_khz = st->current_sample_rate / 1000;
+ int i = 0;
+ u16 pendbc;
+ u32 tsmr, acr;
+
+ if (!state) {
+ /* disabling touch IRQs and setting mode to no touch enabled */
+ at91_adc_writel(st, AT91_SAMA5D2_IDR,
+ AT91_SAMA5D2_IER_PEN | AT91_SAMA5D2_IER_NOPEN);
+ at91_adc_writel(st, AT91_SAMA5D2_TSMR, 0);
+ return 0;
+ }
+ /*
+ * debounce time is in microseconds, we need it in milliseconds to
+ * multiply with kilohertz, so, divide by 1000, but after the multiply.
+ * round up to make sure pendbc is at least 1
+ */
+ pendbc = round_up(TOUCH_PEN_DETECT_DEBOUNCE_US * clk_khz / 1000, 1);
+
+ /* get the required exponent */
+ while (pendbc >> i++)
+ ;
+
+ pendbc = i;
+
+ tsmr = AT91_SAMA5D2_TSMR_TSMODE_4WIRE_PRESS;
+
+ tsmr |= AT91_SAMA5D2_TSMR_TSAV(1) & AT91_SAMA5D2_TSMR_TSAV_MASK;
+ tsmr |= AT91_SAMA5D2_TSMR_PENDBC(pendbc) &
+ AT91_SAMA5D2_TSMR_PENDBC_MASK;
+ tsmr |= AT91_SAMA5D2_TSMR_NOTSDMA;
+ tsmr |= AT91_SAMA5D2_TSMR_PENDET_ENA;
+ tsmr |= AT91_SAMA5D2_TSMR_TSFREQ(1) & AT91_SAMA5D2_TSMR_TSFREQ_MASK;
+
+ at91_adc_writel(st, AT91_SAMA5D2_TSMR, tsmr);
+
+ acr = at91_adc_readl(st, AT91_SAMA5D2_ACR);
+ acr &= ~AT91_SAMA5D2_ACR_PENDETSENS_MASK;
+ acr |= 0x02 & AT91_SAMA5D2_ACR_PENDETSENS_MASK;
+ at91_adc_writel(st, AT91_SAMA5D2_ACR, acr);
+
+ /* Sample Period Time = (TRGPER + 1) / ADCClock */
+ st->touch_st.sample_period_val = round_up((TOUCH_SAMPLE_PERIOD_US *
+ clk_khz / 1000) - 1, 1);
+ /* enable pen detect IRQ */
+ at91_adc_writel(st, AT91_SAMA5D2_IER, AT91_SAMA5D2_IER_PEN);
+
+ return 0;
+}
+
+static int at91_adc_touch_trigger_validate_device(struct iio_trigger *trig,
+ struct iio_dev *indio_dev)
+{
+ /* the touch trigger cannot be used with a buffer */
+ return -EBUSY;
+}
+
+static int at91_adc_configure_touch_trigger(struct iio_trigger *trig,
+ bool state)
+{
+ struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
+ struct at91_adc_state *st = iio_priv(indio_dev);
+ int ret = 0;
+
+ /*
+ * If we configure this with the IRQ enabled, the pen detected IRQ
+ * might fire before we finish setting all up, and the IRQ handler
+ * might misbehave. Better to reenable the IRQ after we are done
+ */
+ disable_irq_nosync(st->irq);
+
+ mutex_lock(&st->lock);
+ if (state) {
+ ret = iio_buffer_enabled(indio_dev);
+ if (ret) {
+ dev_dbg(&indio_dev->dev, "trigger is currently in use\n");
+ ret = -EBUSY;
+ goto configure_touch_unlock_exit;
+ }
+ }
+ at91_adc_configure_touch(st, state);
+ st->touch_requested = state;
+
+configure_touch_unlock_exit:
+ enable_irq(st->irq);
+ mutex_unlock(&st->lock);
+ return ret;
+}
+
static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state)
{
struct iio_dev *indio = iio_trigger_get_drvdata(trig);
@@ -390,12 +594,27 @@ static int at91_adc_reenable_trigger(struct iio_trigger *trig)
return 0;
}
+static int at91_adc_reenable_touch_trigger(struct iio_trigger *trig)
+{
+ struct iio_dev *indio = iio_trigger_get_drvdata(trig);
+ struct at91_adc_state *st = iio_priv(indio);
+
+ enable_irq(st->irq);
+
+ return 0;
+}
static const struct iio_trigger_ops at91_adc_trigger_ops = {
.set_trigger_state = &at91_adc_configure_trigger,
.try_reenable = &at91_adc_reenable_trigger,
.validate_device = iio_trigger_validate_own_device,
};
+static const struct iio_trigger_ops at91_adc_touch_trigger_ops = {
+ .set_trigger_state = &at91_adc_configure_touch_trigger,
+ .try_reenable = &at91_adc_reenable_touch_trigger,
+ .validate_device = &at91_adc_touch_trigger_validate_device,
+};
+
static int at91_adc_dma_size_done(struct at91_adc_state *st)
{
struct dma_tx_state state;
@@ -490,6 +709,23 @@ static int at91_adc_dma_start(struct iio_dev *indio_dev)
return 0;
}
+static int at91_adc_buffer_preenable(struct iio_dev *indio_dev)
+{
+ struct at91_adc_state *st = iio_priv(indio_dev);
+ int ret;
+
+ /* have to make sure nobody is requesting the trigger right now */
+ mutex_lock(&st->lock);
+ ret = st->touch_requested;
+ mutex_unlock(&st->lock);
+
+ /*
+ * if the trigger is used by the touchscreen,
+ * we must return an error
+ */
+ return ret ? -EBUSY : 0;
+}
+
static int at91_adc_buffer_postenable(struct iio_dev *indio_dev)
{
int ret;
@@ -538,6 +774,7 @@ static int at91_adc_buffer_predisable(struct iio_dev *indio_dev)
}
static const struct iio_buffer_setup_ops at91_buffer_setup_ops = {
+ .preenable = &at91_adc_buffer_preenable,
.postenable = &at91_adc_buffer_postenable,
.predisable = &at91_adc_buffer_predisable,
};
@@ -555,7 +792,11 @@ static struct iio_trigger *at91_adc_allocate_trigger(struct iio_dev *indio,
trig->dev.parent = indio->dev.parent;
iio_trigger_set_drvdata(trig, indio);
- trig->ops = &at91_adc_trigger_ops;
+
+ if (strcmp(trigger_name, AT91_ADC_TOUCH_TRIG_SHORTNAME))
+ trig->ops = &at91_adc_trigger_ops;
+ else
+ trig->ops = &at91_adc_touch_trigger_ops;
ret = devm_iio_trigger_register(&indio->dev, trig);
if (ret)
@@ -571,7 +812,16 @@ static int at91_adc_trigger_init(struct iio_dev *indio)
st->trig = at91_adc_allocate_trigger(indio, st->selected_trig->name);
if (IS_ERR(st->trig)) {
dev_err(&indio->dev,
- "could not allocate trigger\n");
+ "could not allocate trigger %s\n",
+ st->selected_trig->name);
+ return PTR_ERR(st->trig);
+ }
+
+ st->touch_st.trig = at91_adc_allocate_trigger(indio,
+ AT91_ADC_TOUCH_TRIG_SHORTNAME);
+ if (IS_ERR(st->trig)) {
+ dev_err(&indio->dev, "could not allocate trigger"
+ AT91_ADC_TOUCH_TRIG_SHORTNAME "\n");
return PTR_ERR(st->trig);
}
@@ -703,6 +953,8 @@ static void at91_adc_setup_samp_freq(struct at91_adc_state *st, unsigned freq)
dev_dbg(&indio_dev->dev, "freq: %u, startup: %u, prescal: %u\n",
freq, startup, prescal);
+
+ st->current_sample_rate = freq;
}
static unsigned at91_adc_get_sample_freq(struct at91_adc_state *st)
@@ -718,23 +970,77 @@ static unsigned at91_adc_get_sample_freq(struct at91_adc_state *st)
return f_adc;
}
+static irqreturn_t at91_adc_pen_detect_interrupt(struct at91_adc_state *st)
+{
+ at91_adc_writel(st, AT91_SAMA5D2_IDR, AT91_SAMA5D2_IER_PEN);
+ at91_adc_writel(st, AT91_SAMA5D2_IER, AT91_SAMA5D2_IER_NOPEN |
+ AT91_SAMA5D2_IER_XRDY | AT91_SAMA5D2_IER_YRDY |
+ AT91_SAMA5D2_IER_PRDY);
+ at91_adc_writel(st, AT91_SAMA5D2_TRGR,
+ AT91_SAMA5D2_TRGR_TRGMOD_PERIODIC |
+ AT91_SAMA5D2_TRGR_TRGPER(st->touch_st.sample_period_val));
+ st->touch_st.touching = true;
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t at91_adc_no_pen_detect_interrupt(struct at91_adc_state *st)
+{
+ at91_adc_writel(st, AT91_SAMA5D2_TRGR, 0);
+ at91_adc_writel(st, AT91_SAMA5D2_IDR, AT91_SAMA5D2_IER_NOPEN |
+ AT91_SAMA5D2_IER_XRDY | AT91_SAMA5D2_IER_YRDY |
+ AT91_SAMA5D2_IER_PRDY);
+ st->touch_st.touching = false;
+
+ disable_irq_nosync(st->irq);
+ iio_trigger_poll(st->touch_st.trig);
+
+ at91_adc_writel(st, AT91_SAMA5D2_IER, AT91_SAMA5D2_IER_PEN);
+
+ return IRQ_HANDLED;
+}
+
static irqreturn_t at91_adc_interrupt(int irq, void *private)
{
struct iio_dev *indio = private;
struct at91_adc_state *st = iio_priv(indio);
u32 status = at91_adc_readl(st, AT91_SAMA5D2_ISR);
u32 imr = at91_adc_readl(st, AT91_SAMA5D2_IMR);
+ u32 rdy_mask = AT91_SAMA5D2_IER_XRDY | AT91_SAMA5D2_IER_YRDY |
+ AT91_SAMA5D2_IER_PRDY;
if (!(status & imr))
return IRQ_NONE;
- if (iio_buffer_enabled(indio) && !st->dma_st.dma_chan) {
+ if (st->touch_requested && (status & AT91_SAMA5D2_IER_PEN)) {
+ /* pen detected IRQ */
+ return at91_adc_pen_detect_interrupt(st);
+ } else if (st->touch_requested && (status & AT91_SAMA5D2_IER_NOPEN)) {
+ /* nopen detected IRQ */
+ return at91_adc_no_pen_detect_interrupt(st);
+ } else if (st->touch_requested && (status & AT91_SAMA5D2_ISR_PENS) &&
+ ((status & rdy_mask) == rdy_mask)) {
+ /* periodic trigger IRQ - during pen sense */
+ disable_irq_nosync(irq);
+ iio_trigger_poll(st->touch_st.trig);
+ } else if ((st->touch_requested && (status & AT91_SAMA5D2_ISR_PENS))) {
+ /*
+ * touching, but the measurements are not ready yet.
+ * read and ignore.
+ */
+ status = at91_adc_readl(st, AT91_SAMA5D2_XPOSR);
+ status = at91_adc_readl(st, AT91_SAMA5D2_YPOSR);
+ status = at91_adc_readl(st, AT91_SAMA5D2_PRESSR);
+ } else if (iio_buffer_enabled(indio) && !st->dma_st.dma_chan) {
+ /* buffered trigger without DMA */
disable_irq_nosync(irq);
iio_trigger_poll(indio->trig);
} else if (iio_buffer_enabled(indio) && st->dma_st.dma_chan) {
+ /* buffered trigger with DMA - should not happen */
disable_irq_nosync(irq);
WARN(true, "Unexpected irq occurred\n");
} else if (!iio_buffer_enabled(indio)) {
+ /* software requested conversion */
st->conversion_value = at91_adc_readl(st, st->chan->address);
st->conversion_done = true;
wake_up_interruptible(&st->wq_data_available);
@@ -742,6 +1048,96 @@ static irqreturn_t at91_adc_interrupt(int irq, void *private)
return IRQ_HANDLED;
}
+static u32 at91_adc_touch_x_pos(struct at91_adc_state *st)
+{
+ u32 xscale, val;
+ u32 x, xpos;
+
+ /* x position = (x / xscale) * max, max = 2^MAX_POS_BITS - 1 */
+ val = at91_adc_readl(st, AT91_SAMA5D2_XPOSR);
+ if (!val)
+ dev_dbg(&iio_priv_to_dev(st)->dev, "x_pos is 0\n");
+
+ xpos = val & XYZ_MASK;
+ x = (xpos << MAX_POS_BITS) - xpos;
+ xscale = (val >> 16) & XYZ_MASK;
+ if (xscale == 0) {
+ dev_err(&iio_priv_to_dev(st)->dev, "xscale is 0\n");
+ return 0;
+ }
+ x /= xscale;
+ st->touch_st.x_pos = x;
+
+ return x;
+}
+
+static u32 at91_adc_touch_y_pos(struct at91_adc_state *st)
+{
+ u32 yscale, val;
+ u32 y, ypos;
+
+ /* y position = (y / yscale) * max, max = 2^MAX_POS_BITS - 1 */
+ val = at91_adc_readl(st, AT91_SAMA5D2_YPOSR);
+ ypos = val & XYZ_MASK;
+ y = (ypos << MAX_POS_BITS) - ypos;
+ yscale = (val >> 16) & XYZ_MASK;
+
+ if (yscale == 0)
+ return 0;
+
+ y /= yscale;
+
+ return y;
+}
+
+static u32 at91_adc_touch_pressure(struct at91_adc_state *st)
+{
+ u32 val, z1, z2;
+ u32 pres;
+ u32 rxp = 1;
+ u32 factor = 1000;
+
+ /* calculate the pressure */
+ val = at91_adc_readl(st, AT91_SAMA5D2_PRESSR);
+ z1 = val & XYZ_MASK;
+ z2 = (val >> 16) & XYZ_MASK;
+
+ if (z1 != 0)
+ pres = rxp * (st->touch_st.x_pos * factor / 1024) *
+ (z2 * factor / z1 - factor) /
+ factor;
+ else
+ pres = 0xFFFFFFFF; /* no pen contact */
+
+ return pres;
+}
+
+static int at91_adc_read_position(struct at91_adc_state *st, int chan, int *val)
+{
+ if (!st->touch_st.touching)
+ return -ENODATA;
+ if (chan == AT91_SAMA5D2_TOUCH_X_CHAN_IDX)
+ *val = at91_adc_touch_x_pos(st);
+ else if (chan == AT91_SAMA5D2_TOUCH_Y_CHAN_IDX)
+ *val = at91_adc_touch_y_pos(st);
+ else
+ return -ENODATA;
+
+ return IIO_VAL_INT;
+}
+
+static int at91_adc_read_pressure(struct at91_adc_state *st, int chan, int *val)
+{
+ if (!st->touch_st.touching)
+ return -ENODATA;
+ if (chan == AT91_SAMA5D2_TOUCH_P_CHAN_IDX)
+ *val = at91_adc_touch_pressure(st);
+ else
+ return -ENODATA;
+
+ return IIO_VAL_INT;
+}
+
static int at91_adc_read_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
int *val, int *val2, long mask)
@@ -752,11 +1148,38 @@ static int at91_adc_read_raw(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_RAW:
+ mutex_lock(&st->lock);
+
+ if (chan->type == IIO_POSITION) {
+ ret = at91_adc_read_position(st, chan->channel, val);
+ mutex_unlock(&st->lock);
+ return ret;
+ }
+ if (chan->type == IIO_PRESSURE) {
+ ret = at91_adc_read_pressure(st, chan->channel, val);
+ mutex_unlock(&st->lock);
+ return ret;
+ }
+ /* if we using touch, channels 0, 1, 2, 3 are unavailable */
+ if (st->touch_requested && chan->channel <= 3) {
+ mutex_unlock(&st->lock);
+ return -EBUSY;
+ }
+ /*
+ * if we have the periodic trigger set up, we can't use
+ * software trigger either.
+ */
+ if (st->touch_st.touching) {
+ mutex_unlock(&st->lock);
+ return -ENODATA;
+ }
+
/* we cannot use software trigger if hw trigger enabled */
ret = iio_device_claim_direct_mode(indio_dev);
- if (ret)
+ if (ret) {
+ mutex_unlock(&st->lock);
return ret;
- mutex_lock(&st->lock);
+ }
st->chan = chan;
@@ -785,6 +1208,11 @@ static int at91_adc_read_raw(struct iio_dev *indio_dev,
at91_adc_writel(st, AT91_SAMA5D2_IDR, BIT(chan->channel));
at91_adc_writel(st, AT91_SAMA5D2_CHDR, BIT(chan->channel));
+ /*
+ * It is possible that after this conversion, we reuse these
+ * channels for the touchscreen. So, reset the COR now.
+ */
+ at91_adc_writel(st, AT91_SAMA5D2_COR, 0);
/* Needed to ACK the DRDY interruption */
at91_adc_readl(st, AT91_SAMA5D2_LCDR);
@@ -1180,6 +1608,10 @@ static int at91_adc_remove(struct platform_device *pdev)
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
struct at91_adc_state *st = iio_priv(indio_dev);
+ mutex_lock(&st->lock);
+ devm_iio_trigger_unregister(&indio_dev->dev, st->touch_st.trig);
+ mutex_unlock(&st->lock);
+
if (st->selected_trig->hw_trig)
devm_iio_trigger_unregister(&indio_dev->dev, st->trig);
@@ -1245,6 +1677,11 @@ static __maybe_unused int at91_adc_resume(struct device *dev)
if (iio_buffer_enabled(indio_dev))
at91_adc_configure_trigger(st->trig, true);
+ mutex_lock(&st->lock);
+ if (st->touch_requested)
+ at91_adc_configure_touch_trigger(st->touch_st.trig, true);
+ mutex_unlock(&st->lock);
+
return 0;
vref_disable_resume:
--
2.7.4
^ permalink raw reply related
* [PATCH 13/14] input: touchscreen: sama5d2_rts: SAMA5D2 Resistive touchscreen driver
From: Eugen Hristev @ 2017-12-22 15:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1513955241-10985-1-git-send-email-eugen.hristev@microchip.com>
This is the implementation of the Microchip SAMA5D2 SOC resistive
touchscreen driver.
The driver registers an input device and connects to the give IIO device
from devicetree. It requires an IIO trigger (acting as a consumer) and
three IIO channels : one for X position, one for Y position and one
for pressure.
It the reports the values to the input subsystem.
Some parts of this driver are based on the initial original work by
Mohamed Jamsheeth Hajanajubudeen and Bandaru Venkateswara Swamy
Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
drivers/input/touchscreen/Kconfig | 13 ++
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/sama5d2_rts.c | 287 ++++++++++++++++++++++++++++++++
3 files changed, 301 insertions(+)
create mode 100644 drivers/input/touchscreen/sama5d2_rts.c
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 64b30fe..db8f541 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -126,6 +126,19 @@ config TOUCHSCREEN_ATMEL_MXT_T37
Say Y here if you want support to output data from the T37
Diagnostic Data object using a V4L device.
+config TOUCHSCREEN_SAMA5D2
+ tristate "Microchip SAMA5D2 resistive touchscreen support"
+ depends on ARCH_AT91
+ depends on AT91_SAMA5D2_ADC
+ help
+ Say Y here if you have 4-wire touchscreen connected
+ to ADC Controller on your SAMA5D2 Microchip SoC.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called sama5d2_rts.
+
config TOUCHSCREEN_AUO_PIXCIR
tristate "AUO in-cell touchscreen using Pixcir ICs"
depends on I2C
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 850c156..9a2772e 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_TOUCHSCREEN_AD7879_SPI) += ad7879-spi.o
obj-$(CONFIG_TOUCHSCREEN_ADS7846) += ads7846.o
obj-$(CONFIG_TOUCHSCREEN_AR1021_I2C) += ar1021_i2c.o
obj-$(CONFIG_TOUCHSCREEN_ATMEL_MXT) += atmel_mxt_ts.o
+obj-$(CONFIG_TOUCHSCREEN_SAMA5D2) += sama5d2_rts.o
obj-$(CONFIG_TOUCHSCREEN_AUO_PIXCIR) += auo-pixcir-ts.o
obj-$(CONFIG_TOUCHSCREEN_BU21013) += bu21013_ts.o
obj-$(CONFIG_TOUCHSCREEN_CHIPONE_ICN8318) += chipone_icn8318.o
diff --git a/drivers/input/touchscreen/sama5d2_rts.c b/drivers/input/touchscreen/sama5d2_rts.c
new file mode 100644
index 0000000..e2ae413
--- /dev/null
+++ b/drivers/input/touchscreen/sama5d2_rts.c
@@ -0,0 +1,287 @@
+/*
+ * Microchip resistive touchscreen (RTS) driver for SAMA5D2.
+ *
+ * Copyright (C) 2017 Microchip Technology,
+ * Author: Eugen Hristev <eugen.hristev@microchip.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+#include <linux/input.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/iio/consumer.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+
+#define DRIVER_NAME "sama5d2_rts"
+#define MAX_POS_MASK GENMASK(11, 0)
+#define AT91_RTS_DEFAULT_PRESSURE_THRESHOLD 10000
+
+/**
+ * at91_rts - at91 resistive touchscreen information struct
+ * @input: the input device structure that we register
+ * @chan_x: X channel to IIO device to get position on X axis
+ * @chan_y: Y channel to IIO device to get position on Y axis
+ * @chan_pressure: pressure channel to IIO device to get pressure
+ * @trig: trigger to IIO device to register to for polling
+ * @rts_pf: pollfunc for the trigger to be called by IIO dev
+ * @pressure_threshold: number representing the threshold for the pressure
+ * @adc_connected: to know if adc device is connected
+ * @workq: to defer computations to this work queue for reporting
+ */
+struct at91_rts {
+ struct input_dev *input;
+ struct iio_channel *chan_x, *chan_y, *chan_pressure;
+ struct iio_trigger *trig;
+ struct iio_poll_func *rts_pf;
+ u32 pressure_threshold;
+ bool adc_connected;
+ struct work_struct workq;
+};
+
+static irqreturn_t at91_rts_trigger_handler(int irq, void *p)
+{
+ struct at91_rts *st = ((struct iio_poll_func *)p)->p;
+
+ schedule_work(&st->workq);
+
+ return IRQ_HANDLED;
+}
+
+static void at91_rts_workq_handler(struct work_struct *workq)
+{
+ struct at91_rts *st = container_of(workq, struct at91_rts, workq);
+ unsigned int x, y, press;
+ int ret;
+
+ /* read the channels, if all good, report touch */
+ ret = iio_read_channel_raw(st->chan_x, &x);
+ if (ret < 0)
+ goto at91_rts_workq_handler_end_touch;
+
+ ret = iio_read_channel_raw(st->chan_y, &y);
+ if (ret < 0)
+ goto at91_rts_workq_handler_end_touch;
+
+ ret = iio_read_channel_raw(st->chan_pressure, &press);
+ if (ret < 0)
+ goto at91_rts_workq_handler_end_touch;
+
+ /* if pressure too low, don't report */
+ if (press > st->pressure_threshold)
+ goto at91_rts_workq_handler_exit;
+
+ input_report_abs(st->input, ABS_X, x);
+ input_report_abs(st->input, ABS_Y, y);
+ input_report_abs(st->input, ABS_PRESSURE, press);
+ input_report_key(st->input, BTN_TOUCH, 1);
+ input_sync(st->input);
+
+ iio_trigger_notify_done(st->trig);
+ return;
+
+at91_rts_workq_handler_end_touch:
+ /* report end of touch */
+ input_report_key(st->input, BTN_TOUCH, 0);
+ input_sync(st->input);
+at91_rts_workq_handler_exit:
+ iio_trigger_notify_done(st->trig);
+}
+
+static int at91_rts_open(struct input_dev *dev)
+{
+ int ret;
+ struct at91_rts *st = input_get_drvdata(dev);
+
+ /* avoid multiple initialization in case touchscreen is opened again */
+ if (st->adc_connected)
+ return 0;
+
+ /*
+ * First, look for the channels. It is possible that the ADC device
+ * did not probe yet, but we already probed, so we returning probe defer
+ * doesn't make much sense.
+ */
+ st->chan_x = iio_channel_get(dev->dev.parent, "x");
+ if (IS_ERR_OR_NULL(st->chan_x)) {
+ dev_err(dev->dev.parent, "cannot get X channel from ADC");
+ ret = PTR_ERR(st->chan_x);
+ goto at91_rts_open_free_chan;
+ }
+
+ st->chan_y = iio_channel_get(dev->dev.parent, "y");
+ if (IS_ERR_OR_NULL(st->chan_y)) {
+ dev_err(dev->dev.parent, "cannot get Y channel from ADC");
+ ret = PTR_ERR(st->chan_y);
+ goto at91_rts_open_free_chan;
+ }
+
+ st->chan_pressure = iio_channel_get(dev->dev.parent, "pressure");
+ if (IS_ERR_OR_NULL(st->chan_pressure)) {
+ dev_err(dev->dev.parent, "cannot get pressure channel from ADC");
+ ret = PTR_ERR(st->chan_pressure);
+ goto at91_rts_open_free_chan;
+ }
+
+ /* look for the trigger in device tree */
+ st->trig = iio_trigger_find(dev->dev.parent, NULL);
+ if (IS_ERR_OR_NULL(st->trig)) {
+ dev_err(dev->dev.parent, "cannot get trigger from ADC");
+ ret = PTR_ERR(st->trig);
+ goto at91_rts_open_free_chan;
+ }
+
+ /* allocate a pollfunc for the trigger */
+ st->rts_pf = iio_alloc_pollfunc(at91_rts_trigger_handler, NULL,
+ IRQF_ONESHOT, NULL,
+ dev->dev.parent->of_node->name);
+ if (!st->rts_pf) {
+ ret = -ENOMEM;
+ dev_err(dev->dev.parent, "cannot allocate trigger pollfunc");
+ goto at91_rts_open_free_chan;
+ }
+
+ iio_pollfunc_set_private_data(st->rts_pf, st);
+
+ /*
+ * Attach the pollfunc to the trigger. This will also call the
+ * configure function to enable the trigger
+ */
+ ret = iio_trigger_attach_poll_func(st->trig, st->rts_pf);
+ if (ret)
+ goto at91_rts_open_dealloc_pf;
+
+ dev_dbg(dev->dev.parent, "channels found, attached to trigger");
+
+ st->adc_connected = true;
+ return 0;
+
+at91_rts_open_dealloc_pf:
+ iio_dealloc_pollfunc(st->rts_pf);
+at91_rts_open_free_chan:
+ if (!IS_ERR_OR_NULL(st->chan_x))
+ iio_channel_release(st->chan_x);
+ if (!IS_ERR_OR_NULL(st->chan_y))
+ iio_channel_release(st->chan_y);
+ if (!IS_ERR_OR_NULL(st->chan_pressure))
+ iio_channel_release(st->chan_pressure);
+ /*
+ * Avoid keeping old values in channel pointers. in case some channel
+ * failed and we reopen them, and now fail, we will have invalid values
+ * to release. So write them as NULL now.
+ */
+ st->chan_x = NULL;
+ st->chan_y = NULL;
+ st->chan_pressure = NULL;
+ return ret;
+}
+
+static void at91_rts_close(struct input_dev *dev)
+{
+ struct at91_rts *st = input_get_drvdata(dev);
+
+ if (!st->adc_connected)
+ return;
+
+ iio_trigger_detach_poll_func(st->trig, st->rts_pf);
+ iio_dealloc_pollfunc(st->rts_pf);
+
+ if (!IS_ERR_OR_NULL(st->chan_x))
+ iio_channel_release(st->chan_x);
+ if (!IS_ERR_OR_NULL(st->chan_y))
+ iio_channel_release(st->chan_y);
+ if (!IS_ERR_OR_NULL(st->chan_pressure))
+ iio_channel_release(st->chan_pressure);
+
+ st->adc_connected = false;
+}
+
+static int at91_rts_probe(struct platform_device *pdev)
+{
+ int ret;
+ struct at91_rts *st;
+ struct input_dev *input;
+ struct device *dev = &pdev->dev;
+ struct device_node *node = dev->of_node;
+
+ st = devm_kzalloc(dev, sizeof(struct at91_rts), GFP_KERNEL);
+ if (!st)
+ return -ENOMEM;
+ st->adc_connected = false;
+
+ INIT_WORK(&st->workq, at91_rts_workq_handler);
+
+ input = devm_input_allocate_device(dev);
+ if (!input) {
+ dev_err(dev, "failed to allocate input device\n");
+ return -ENOMEM;
+ }
+
+ ret = of_property_read_u32(node, "microchip,pressure-threshold",
+ &st->pressure_threshold);
+ if (ret < 0) {
+ dev_dbg(dev, "can't get touchscreen pressure threshold property.\n");
+ st->pressure_threshold = AT91_RTS_DEFAULT_PRESSURE_THRESHOLD;
+ }
+
+ input->name = DRIVER_NAME;
+ input->id.bustype = BUS_HOST;
+ input->dev.parent = &pdev->dev;
+ input->open = at91_rts_open;
+ input->close = at91_rts_close;
+
+ input_set_abs_params(input, ABS_X, 0, MAX_POS_MASK - 1, 0, 0);
+ input_set_abs_params(input, ABS_Y, 0, MAX_POS_MASK, 0, 0);
+ input_set_abs_params(input, ABS_PRESSURE, 0, 0xffffff, 0, 0);
+
+ input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
+ input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
+
+ st->input = input;
+ input_set_drvdata(input, st);
+
+ ret = input_register_device(input);
+ if (ret) {
+ dev_err(dev, "failed to register input device: %d", ret);
+ return ret;
+ }
+
+ platform_set_drvdata(pdev, st);
+
+ dev_info(dev, "probed successfully\n");
+ return 0;
+}
+
+static int at91_rts_remove(struct platform_device *pdev)
+{
+ struct at91_rts *st = platform_get_drvdata(pdev);
+
+ input_unregister_device(st->input);
+
+ return 0;
+}
+
+static const struct of_device_id at91_rts_of_match[] = {
+ {
+ .compatible = "microchip,sama5d2-resistive-touch",
+ }, {
+ /* sentinel */
+ },
+};
+MODULE_DEVICE_TABLE(of, at91_rts_of_match);
+
+static struct platform_driver atmel_rts_driver = {
+ .probe = at91_rts_probe,
+ .remove = at91_rts_remove,
+ .driver = {
+ .name = DRIVER_NAME,
+ .of_match_table = of_match_ptr(at91_rts_of_match),
+ },
+};
+
+module_platform_driver(atmel_rts_driver);
+
+MODULE_AUTHOR("Eugen Hristev <eugen.hristev@microchip.com>");
+MODULE_DESCRIPTION("Microchip SAMA5D2 Resistive Touch Driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4
^ permalink raw reply related
* [PATCH 14/14] ARM: dts: at91: sama5d2: Add resistive touch device
From: Eugen Hristev @ 2017-12-22 15:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1513955241-10985-1-git-send-email-eugen.hristev@microchip.com>
Add the resistive touchscreen device, and the cell numbers to the
ADC device.
Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
arch/arm/boot/dts/sama5d2.dtsi | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi
index b1a26b4..30b3797 100644
--- a/arch/arm/boot/dts/sama5d2.dtsi
+++ b/arch/arm/boot/dts/sama5d2.dtsi
@@ -402,7 +402,6 @@
compatible = "atmel,hlcdc-display-controller";
#address-cells = <1>;
#size-cells = <0>;
-
port at 0 {
#address-cells = <1>;
#size-cells = <0>;
@@ -1431,6 +1430,17 @@
atmel,max-sample-rate-hz = <20000000>;
atmel,startup-time-ms = <4>;
atmel,trigger-edge-type = <IRQ_TYPE_EDGE_RISING>;
+ #io-channel-cells = <1>;
+ #io-trigger-cells = <1>;
+ status = "disabled";
+ };
+
+ resistive_touch: resistive-touch {
+ compatible = "microchip,sama5d2-resistive-touch";
+ io-channels = <&adc 19>, <&adc 20>, <&adc 21>;
+ io-channel-names = "x", "y", "pressure";
+ io-triggers = <&adc 1>;
+ microchip,pressure-threshold = <10000>;
status = "disabled";
};
--
2.7.4
^ permalink raw reply related
* [PATCH v2 0/8] arm64: 52-bit physical address support
From: Catalin Marinas @ 2017-12-22 15:22 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
That's v2 of Kristina's 52-bit PA series, posted here:
http://lkml.kernel.org/r/1513184845-8711-1-git-send-email-kristina.martsenko at arm.com
I addressed the comments raised on the list and I plan to push it into
-next soon.
Changes in v2:
- Folded patches 7 and 8 from the original series into 1
- Definitions for TCR_IPS_*
- Renamed some asm macros and functions
- __create_hyp_mappings() changed to avoid passing an extra arg
- More code comments
- Added Reviewed/Tested tags I've got so far
Thanks,
Catalin
Kristina Martsenko (8):
arm64: add kconfig symbol to configure physical address size
arm64: limit PA size to supported range
arm64: handle 52-bit addresses in TTBR
arm64: head.S: handle 52-bit PAs in PTEs in early page table setup
arm64: don't open code page table entry creation
arm64: handle 52-bit physical addresses in page table entries
arm64: allow ID map to be extended to 52 bits
arm64: enable 52-bit physical address support
arch/arm/include/asm/kvm_mmu.h | 7 ++
arch/arm64/Kconfig | 29 ++++++++
arch/arm64/include/asm/assembler.h | 36 +++++++++-
arch/arm64/include/asm/kvm_mmu.h | 21 +++++-
arch/arm64/include/asm/mmu_context.h | 20 ++++--
arch/arm64/include/asm/pgalloc.h | 6 +-
arch/arm64/include/asm/pgtable-hwdef.h | 25 ++++++-
arch/arm64/include/asm/pgtable.h | 55 ++++++++++++---
arch/arm64/include/asm/sparsemem.h | 2 +-
arch/arm64/include/asm/sysreg.h | 8 +++
arch/arm64/kernel/head.S | 122 +++++++++++++++++++++------------
arch/arm64/kernel/hibernate-asm.S | 12 ++--
arch/arm64/kernel/hibernate.c | 5 +-
arch/arm64/kvm/hyp-init.S | 26 ++++---
arch/arm64/kvm/hyp/s2-setup.c | 2 +
arch/arm64/mm/mmu.c | 15 ++--
arch/arm64/mm/pgd.c | 8 +++
arch/arm64/mm/proc.S | 19 ++---
virt/kvm/arm/arm.c | 2 +-
virt/kvm/arm/mmu.c | 10 ++-
20 files changed, 323 insertions(+), 107 deletions(-)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox