Re: [PATCH v16 1/2] drm/tegra: dc: Support memory bandwidth management

[Michał Mirosław]

t; + /*
> +  * Then we calculate maximum bandwidth of each plane state.
> +  * The bandwidth includes the plane BW + BW of the "simultaneously"
> +  * overlapping planes, where "simultaneously" means areas where DC
> +  * fetches from the planes simultaneously during of scan-out process.
> +  *
> +  * For example, if plane A overlaps with planes B and C, but B and C
> +  * don't overlap, then the peak bandwidth will be either in area where
> +  * A-and-B or A-and-C planes overlap.
> +  *
> +  * The plane_peak_bw[] contains peak memory bandwidth values of
> +  * each plane, this information is needed by interconnect provider
> +  * in order to set up latency allowness based on the peak BW, see
> +  * tegra_crtc_update_memory_bandwidth().
> +  */
> + for (i = 0; i < ARRAY_SIZE(plane_peak_bw); i++) {
> + overlap_bw = 0;
> +
> + for_each_set_bit(k, _mask[i], 3) {
> + if (k == i)
> + continue;
> +
> + if (all_planes_overlap_simultaneously)
> + overlap_bw += plane_peak_bw[k];
> + else
> + overlap_bw = max(overlap_bw, plane_peak_bw[k]);
> + }
> +
> + new_dc_state->plane_peak_bw[i] = plane_peak_bw[i] + overlap_bw;
> +
> + /*
> +  * If plane's peak bandwidth changed (for example plane isn't
> +  * overlapped anymore) and plane isn't in the atomic state,
> +  * then add plane to the state in order to have the bandwidth
> +  * updated.
> +  */
> + if (old_dc_state->plane_peak_bw[i] !=
> + new_dc_state->plane_peak_bw[i]) {
> + plane = tegra_crtc_get_plane_by_index(crtc, i);
> + if (!plane)
> + continue;
> +
> + plane_state = drm_atomic_get_plane_state(state, plane);
> + if (IS_ERR(plane_state))
> + return PTR_ERR(plane_state);
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int tegra_crtc_atomic_check(struct drm_crtc *crtc,
> +struct drm_atomic_state *state)
> +{
> + int err;
> +
> + err = tegra_crtc_calculate_memory_bandwidth(crtc, state);
> + if (err)
> + return err;
> +
> + return 0;
> +}
> +
> +void tegra_crtc_atomic_post_commit(struct drm_crtc *crtc,
> +struct drm_atomic_state *state)
> +{
> + /*
> +  * Display bandwidth is allowed to go down only once hardware state
> +  * is known to be armed, i.e. state was committed and VBLANK event
> +  * received.
> +  */
> + tegra_crtc_update_memory_bandwidth(crtc, state, false);
> +}
> +
>  static const struct drm_crtc_helper_funcs tegra_crtc_helper_funcs = {
> + .atomic_check = tegra_crtc_atomic_check,
>   .atomic_begin = tegra_crtc_atomic_begin,
>   .atomic_flush = tegra_crtc_atomic_flush,
>   .atomic_enable = tegra_crtc_atomic_enable,
> @@ -2257,7 +2597,9 @@ static const struct tegra_dc_soc_info 
> tegra20_dc_soc_info = {
>   .overlay_formats = tegra20_overlay_formats,
>   .modifiers = tegra20_modifiers,
>   .has_win_a_without_filters = true,
> + .has_win_b_vfilter_mem_client = true,
>   .has_win_c_without_vert_filter = true,
> + .plane_tiled_memory_bandwidth_x2 = false,
>  };
>  
>  static const struct tegra_dc_soc_info tegra30_dc_soc_info = {
> @@ -2276,7 +2618,9 @@ static const struct tegra_dc_soc_info 
> tegra30_dc_soc_info = {
>   .overlay_formats = tegra20_overlay_formats,
>   .modifiers = tegra20_modifiers,
>   .has_win_a_without_filters = false,
> + .has_win_b_vfilter_mem_client = true,
>   .has_win_c_without_vert_filter = false,
> + .plane_tiled_memory_bandwidth_x2 = true,
>  };
>  
>  static const struct tegra_dc_soc_info tegra114_dc_soc_info = {
> @@ -2295,7 +2639,9 @@ static const struct tegra_dc_soc_info 
> tegra114_dc_soc_info = {
>   .overlay_formats = tegra114_overlay_formats,
>   .modifiers = tegra20_modifiers,
>   .has_win_a_without_filters = false,
> + .has_win_b_vfilter_mem_client = false,
>   .has_win_c_without_vert_filter = false,
> + .plane_tiled_memory_bandwidth_x2 = true,
>  };
>  
>  static const struct tegra_dc_soc_info tegra124_dc_soc_info = {
> @@ -2314,7 +2660,9 @@ static const struct tegra_dc_soc_info 
> tegra124_dc_soc_info = {
>   .overlay_formats = tegra124_overlay_formats,
>   .modifiers = tegra124_modifiers,
>   .has_win_a_without_filters = false,
> + .has_win_b_vfilter_mem_client = false,
>   .has_win_c_without_vert_filter = false,
> + .plane_tiled_memory_bandwidth_x2 = false,
>  };
>  
>  static const struct tegra_dc_soc_info tegra210_dc_soc_info = {
> @@ -2333,7 +2681,9 @@ static const struct tegra_dc_soc_info 
> tegra210_dc_soc_info = {
>   .overlay_formats = tegra114_overlay_formats,
>   .modifiers = tegra124_modifiers,
>   .has_win_a_without_filters = false,
> + .has_win_b_vfilter_mem_client = false,
>   .has_win_c_without_vert_filter = false,
> + .plane_tiled_memory_bandwidth_x2 = false,
>  };
>  
>  static const struct tegra_windowgroup_soc tegra186_dc_wgrps[] = {
> @@ -2382,6 +2732,7 @@ static const struct tegra_dc_soc_info 
> tegra186_dc_soc_info = {
>   .has_nvdisplay = true,
>   .wgrps = tegra186_dc_wgrps,
>   .num_wgrps = ARRAY_SIZE(tegra186_dc_wgrps),
> + .plane_tiled_memory_bandwidth_x2 = false,
>  };
>  
>  static const struct tegra_windowgroup_soc tegra194_dc_wgrps[] = {
> @@ -2430,6 +2781,7 @@ static const struct tegra_dc_soc_info 
> tegra194_dc_soc_info = {
>   .has_nvdisplay = true,
>   .wgrps = tegra194_dc_wgrps,
>   .num_wgrps = ARRAY_SIZE(tegra194_dc_wgrps),
> + .plane_tiled_memory_bandwidth_x2 = false,
>  };

For globals you will have .x = false by default; I'm not sure those entries
add much value.

Reviewed-by: Michał Mirosław 

Re: [PATCH v5 2/7] clk: tegra: Fix refcounting of gate clocks

[Michał Mirosław]

On Wed, Mar 17, 2021 at 10:30:01PM +0300, Dmitry Osipenko wrote:
> The refcounting of the gate clocks has a bug causing the enable_refcnt
> to underflow when unused clocks are disabled. This happens because clk
> provider erroneously bumps the refcount if clock is enabled at a boot
> time, which it shouldn't be doing, and it does this only for the gate
> clocks, while peripheral clocks are using the same gate ops and the
> peripheral clocks are missing the initial bump. Hence the refcount of
> the peripheral clocks is 0 when unused clocks are disabled and then the
> counter is decremented further by the gate ops, causing the integer
> underflow.
[...]
> diff --git a/drivers/clk/tegra/clk-periph-gate.c 
> b/drivers/clk/tegra/clk-periph-gate.c
> index 4b31beefc9fc..3c4259fec82e 100644
> --- a/drivers/clk/tegra/clk-periph-gate.c
> +++ b/drivers/clk/tegra/clk-periph-gate.c
[...]
> @@ -91,21 +108,28 @@ static void clk_periph_disable(struct clk_hw *hw)
>  
>   spin_lock_irqsave(_ref_lock, flags);
>  
> - gate->enable_refcnt[gate->clk_num]--;
> - if (gate->enable_refcnt[gate->clk_num] > 0) {
> - spin_unlock_irqrestore(_ref_lock, flags);
> - return;
> - }
> + WARN_ON(!gate->enable_refcnt[gate->clk_num]);
> +
> + if (gate->enable_refcnt[gate->clk_num]-- == 1)
> + clk_periph_disable_locked(hw);

Nit: "if (--n == 0)" seems more natural, as you want to call
clk_periph_disable_locked() when the refcount goes down to 0.

[...]
>   /*
> -  * If peripheral is in the APB bus then read the APB bus to
> -  * flush the write operation in apb bus. This will avoid the
> -  * peripheral access after disabling clock
> +  * Some clocks are duplicated and some of them are marked as critical,
> +  * like fuse and fuse_burn for example, thus the enable_refcnt will
> +  * be non-zero here id the "unused" duplicate is disabled by CCF.

s/id/if/ ?

Best Regards
Michał Mirosław

Re: [PATCH v15 1/2] drm/tegra: dc: Support memory bandwidth management

[Michał Mirosław]

lane = tegra_crtc_get_plane_by_index(crtc, i);
> + if (!plane)
> + continue;
> +
> + plane_state = drm_atomic_get_plane_state(state, plane);
> + if (IS_ERR(plane_state))
> + return PTR_ERR(plane_state);
> + }
> + }
[...]

Does it matter to which channel (plane) the peak bw is attached? Would
it still work if the first channel specified max(peak_bw of overlaps)
and others only zeroes?

> + /*
> +  * Horizontal downscale needs a lower memory latency, which roughly
> +  * depends on the scaled width.  Trying to tune latency of a memory
> +  * client alone will likely result in a strong negative impact on
> +  * other memory clients, hence we will request a higher bandwidth
> +  * since latency depends on bandwidth.  This allows to prevent memory
> +  * FIFO underflows for a large plane downscales, meanwhile allowing
> +  * display to share bandwidth fairly with other memory clients.
> +  */
> + if (src_w > dst_w)
> + mul = (src_w - dst_w) * bpp / 2048 + 1;
> + else
> + mul = 1;
[...]

One point is unexplained yet: why is the multiplier proportional to a
*difference* between src and dst widths? Also, I would expect max (worst
case) is pixclock * read_size when src_w/dst_w >= read_size.

BTW, you could move this below and : if (src > dst_w) peak_bandwidth *= ...

> + /* average bandwidth in bytes/s */
> + avg_bandwidth  = (bpp * src_w * src_h * mul + 7) / 8;
> + avg_bandwidth *= drm_mode_vrefresh(_state->mode);
> +
> + /* mode.clock in kHz, peak bandwidth in kbit/s */
> + peak_bandwidth = crtc_state->mode.clock * bpp * mul;
[...]

Best Regards
Michał Mirosław

Re: [PATCH v15 2/2] drm/tegra: dc: Extend debug stats with total number of events

[Michał Mirosław]

On Thu, Mar 11, 2021 at 08:22:55PM +0300, Dmitry Osipenko wrote:
> It's useful to know the total number of underflow events and currently
> the debug stats are getting reset each time CRTC is being disabled. Let's
> account the overall number of events that doesn't get a reset.
[...]

Looks good. It seems independent from the other patch.

Reviewed-by: Michał Mirosław 

Re: [PATCH v13 1/2] drm/tegra: dc: Support memory bandwidth management

[Michał Mirosław]

On Fri, Mar 05, 2021 at 12:45:51AM +0300, Dmitry Osipenko wrote:
> 04.03.2021 02:08, Michał Mirosław пишет:
> > On Tue, Mar 02, 2021 at 03:44:44PM +0300, Dmitry Osipenko wrote:
> >> Display controller (DC) performs isochronous memory transfers, and thus,
> >> has a requirement for a minimum memory bandwidth that shall be fulfilled,
> >> otherwise framebuffer data can't be fetched fast enough and this results
> >> in a DC's data-FIFO underflow that follows by a visual corruption.
[...]
> >> +  /*
> >> +   * Horizontal downscale takes extra bandwidth which roughly depends
> >> +   * on the scaled width.
> >> +   */
> >> +  if (src_w > dst_w)
> >> +  mul = (src_w - dst_w) * bpp / 2048 + 1;
> >> +  else
> >> +  mul = 1;
> > 
> > Does it really need more bandwidth to scale down? Does it read the same
> > data multiple times just to throw it away?
> The hardware isn't optimized for downscale, it indeed takes more
> bandwidth. You'll witness a severe underflow of plane's memory FIFO
> buffer on trying to downscale 1080p plane to 50x50.
[...]

In your example, does it really need 16x the bandwidth compared to
no scaling case?  The naive way to implement downscaling would be to read
all the pixels and only take every N-th.  Maybe the problem is that in
downscaling mode the latency requirements are tighter?  Why would bandwidth
required be proportional to a difference between the widths (instead e.g.
to src/dst or dst*cacheline_size)?

Best Regards
Michał Mirosław

Re: [PATCH v1] Input: elants_i2c - fix division by zero if firmware reports zero phys size

[Michał Mirosław]

On Tue, Mar 02, 2021 at 01:08:24PM +0300, Dmitry Osipenko wrote:
> Touchscreen firmware of ASUS Transformer TF700T reports zeros for the phys
> size. Hence check whether the size is zero and don't set the resolution in
> this case.
> 
> Reported-by: Jasper Korten 
> Signed-off-by: Dmitry Osipenko 
> ---

Rewieved-by: Michał Mirosław 


> Please note that ASUS TF700T isn't yet supported by upstream kernel,
> hence this is not a critical fix.
> 
>  drivers/input/touchscreen/elants_i2c.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/elants_i2c.c 
> b/drivers/input/touchscreen/elants_i2c.c
> index 4c2b579f6c8b..a2e1cc4192b0 100644
> --- a/drivers/input/touchscreen/elants_i2c.c
> +++ b/drivers/input/touchscreen/elants_i2c.c
> @@ -1441,14 +1441,16 @@ static int elants_i2c_probe(struct i2c_client *client,
>  
>   touchscreen_parse_properties(ts->input, true, >prop);
>  
> - if (ts->chip_id == EKTF3624) {
> + if (ts->chip_id == EKTF3624 && ts->phy_x && ts->phy_y) {
>   /* calculate resolution from size */
>   ts->x_res = DIV_ROUND_CLOSEST(ts->prop.max_x, ts->phy_x);
>   ts->y_res = DIV_ROUND_CLOSEST(ts->prop.max_y, ts->phy_y);
>   }
>  
> - input_abs_set_res(ts->input, ABS_MT_POSITION_X, ts->x_res);
> - input_abs_set_res(ts->input, ABS_MT_POSITION_Y, ts->y_res);
> + if (ts->x_res > 0)
> + input_abs_set_res(ts->input, ABS_MT_POSITION_X, ts->x_res);
> + if (ts->y_res > 0)
> + input_abs_set_res(ts->input, ABS_MT_POSITION_Y, ts->y_res);

I would guess that you can tie X and Y in one if, as they are unlikely
to work independently.

Best Regards
Michal Mirosław

Re: [PATCH 06/11] pragma once: convert include/linux/cb710.h

[Michał Mirosław]

On Sun, Feb 28, 2021 at 08:02:10PM +0300, Alexey Dobriyan wrote:
> From 1c4107e55b322dada46879837d4d64841bc5f150 Mon Sep 17 00:00:00 2001
> From: Alexey Dobriyan 
> Date: Tue, 9 Feb 2021 16:56:54 +0300
> Subject: [PATCH 06/11] pragma once: convert include/linux/cb710.h
> 
> This file is concatenation of two files with two include guards.
> Convert it manually.
> 
> Signed-off-by: Alexey Dobriyan 

Acked-by: Michał Mirosław 

Re: [PATCH v13 1/2] drm/tegra: dc: Support memory bandwidth management

[Michał Mirosław]

h the assignments after the following 'if' block.

> + /*
> +  * Tegra30/114 Memory Controller can't interleave DC memory requests
> +  * and DC uses 16-bytes atom for the tiled windows, while DDR3 uses 32
> +  * bytes atom. Hence there is x2 memory overfetch for tiled framebuffer
> +  * and DDR3 on older SoCs.
> +  */
> + if (soc->plane_tiled_memory_bandwidth_x2 &&
> + tegra_state->tiling.mode == TEGRA_BO_TILING_MODE_TILED) {
> + peak_bandwidth *= 2;
> + avg_bandwidth *= 2;
> + }
> +
> + tegra_state->peak_memory_bandwidth = peak_bandwidth;
> + tegra_state->avg_memory_bandwidth = avg_bandwidth;
> +
> + return 0;
> +}

[...]
> +static const char * const tegra_plane_icc_names[] = {
> + "wina", "winb", "winc", "", "", "", "cursor",
> +};
> +
> +int tegra_plane_interconnect_init(struct tegra_plane *plane)
> +{
> + const char *icc_name = tegra_plane_icc_names[plane->index];

Is plane->index guaranteed to be <= 6? I would guess so, but maybe
BUILD_BUG_ON(sizeof(icc_names)==TEGRA_DC_LEGACY_PLANES_NUM) or some
other check could document this?

[...]

Best Regards
Michał Mirosław

Re: [PATCH v2] ptrace: add PTRACE_GET_RSEQ_CONFIGURATION request

[Michał Mirosław]

On Fri, 26 Feb 2021 at 16:32, Mathieu Desnoyers
 wrote:
>
> - On Feb 26, 2021, at 8:51 AM, Piotr Figiel fig...@google.com wrote:
> [...]
> > ---
> > v2:
> > Applied review comments:
> > - changed return value from the ptrace request to the size of the
> >   configuration structure
> > - expanded configuration structure with the flags field and
> >   the rseq abi structure size
> >
> [...]
> > +#define PTRACE_GET_RSEQ_CONFIGURATION0x420f
> > +
> > +struct ptrace_rseq_configuration {
> > + __u64 rseq_abi_pointer;
> > + __u32 rseq_abi_size;
> > + __u32 signature;
> > + __u32 flags;
> > + __u32 pad;
> > +};
> > +
> [...]
> > +#ifdef CONFIG_RSEQ
> > +static long ptrace_get_rseq_configuration(struct task_struct *task,
> > +   unsigned long size, void __user 
> > *data)
> > +{
> > + struct ptrace_rseq_configuration conf = {
> > + .rseq_abi_pointer = (u64)(uintptr_t)task->rseq,
> > + .rseq_abi_size = sizeof(*task->rseq),
> > + .signature = task->rseq_sig,
> > + .flags = 0,
> > + };
> > +
> > + size = min_t(unsigned long, size, sizeof(conf));
> > + if (copy_to_user(data, , size))
> > + return -EFAULT;
> > + return sizeof(conf);
> > +}
>
> I think what Florian was after would be:
>
> struct ptrace_rseq_configuration {
> __u32 size;  /* size of struct ptrace_rseq_configuration */
> __u32 flags;
> __u64 rseq_abi_pointer;
> __u32 signature;
> __u32 pad;
> };
>
> where:
>
> .size = sizeof(struct ptrace_rseq_configuration),
>
> This way, the configuration structure can be expanded in the future. The
> rseq ABI structure is by definition fixed-size, so there is no point in
> having its size here.
>
> Florian, did I understand your request correctly, or am I missing your point ?

In this case returning sizeof(conf) would serve the same purpose, wouldn't it?

Best Regards
Michał Mirosław

[Resent because of HTML mail misfeature...]

Re: [PATCH RFC 4/6] btrfs: Check if the filesystem is has mixed type of devices

[Michał Mirosław]

On Fri, Feb 12, 2021 at 06:26:41PM +, Michal Rostecki wrote:
> On Wed, Feb 10, 2021 at 05:08:05AM +0100, Michał Mirosław wrote:
> > On Tue, Feb 09, 2021 at 09:30:38PM +0100, Michal Rostecki wrote:
> > > From: Michal Rostecki 
> > > 
> > > Add the btrfs_check_mixed() function which checks if the filesystem has
> > > the mixed type of devices (non-rotational and rotational). This
> > > information is going to be used in roundrobin raid1 read policy.a
> > [...]
> > > @@ -669,8 +699,12 @@ static int btrfs_open_one_device(struct 
> > > btrfs_fs_devices *fs_devices,
> > >   }
> > >  
> > >   q = bdev_get_queue(bdev);
> > > - if (!blk_queue_nonrot(q))
> > > + rotating = !blk_queue_nonrot(q);
> > > + device->rotating = rotating;
> > > + if (rotating)
> > >   fs_devices->rotating = true;
> > > + if (!fs_devices->mixed)
> > > + fs_devices->mixed = btrfs_check_mixed(fs_devices, rotating);
> > [...]
> > 
> > Since this is adding to a set, a faster way is:
> > 
> > if (fs_devices->rotating != rotating)
> > fs_devices->mixed = true;
> > 
> > The scan might be necessary on device removal, though.
> Actually, that's not going to work in case of appenging a rotational
> device when all previous devices are non-rotational.
[...]
> Inverting the order of those `if` checks would break the other
> permuitations which start with rotational disks.

But not if you would add:

if (adding first device)
fs_devices->rotating = rotating;

before the checks.

But them, there is a simpler way: count how many rotating vs non-rotating
devices there are while adding them. Like:

rotating ? ++n_rotating : ++n_fixed;

And then on remove you'd have it covered.

Best Regards
Michał Mirosław

Re: [PATCH RFC 6/6] btrfs: Add roundrobin raid1 read policy

[Michał Mirosław]

On Wed, Feb 10, 2021 at 07:23:04PM +, Michal Rostecki wrote:
> On Wed, Feb 10, 2021 at 01:58:15PM +0100, Michał Mirosław wrote:
> > On Wed, Feb 10, 2021 at 12:29:25PM +, Michal Rostecki wrote:
> > > On Wed, Feb 10, 2021 at 05:24:28AM +0100, Michał Mirosław wrote:
> > > > This looks like it effectively decreases queue depth for non-last
> > > > device. After all devices are filled to queue_depth-penalty, only
> > > > a single mirror will be selected for next reads (until a read on
> > > > some other one completes).
> > > > 
> > > 
> > > Good point. And if all devices are going to be filled for longer time,
> > > this function will keep selecting the last one. Maybe I should select
> > > last+1 in that case. Would that address your concern or did you have any
> > > other solution in mind?
> > 
> > The best would be to postpone the selection until one device becomes free
> > again. But if that's not doable, then yes, you could make sure it stays
> > round-robin after filling the queues (the scheduling will loose the
> > "penalty"-driven adjustment though).
> 
> Or another idea - when all the queues are filled, return the mirror
> which has the lowest load (inflight + penalty), even though it's greater
> than queue depth. In that case the scheduling will not lose the penalty
> adjustment and the load is going to be spreaded more fair.
> 
> I'm not sure if postponing the selection is that good idea. I think it's
> better if the request is added to the iosched queue anyway, even if the
> disks' queues are filled, and let the I/O scheduler handle that. The
> length of the iosched queue (nr_requests, attribute of the iosched) is
> usually greater than queue depth (attribute of the devide), which means
> that it's fine to schedule more requests for iosched to handle.
> 
> IMO btrfs should use the information given by iosched only for heuristic
> mirror selection, rather than implement its own throttling logic.
> 
> Does it make sense to you?
> 
> An another idea could be an additional iteration in regard to
> nr_requests, if all load values are greater than queue depths, though it
> might be an overkill. I would prefer to stick to my first idea if
> everyone agrees.

What if iosched could provide an estimate of request's latency? Then
btrfs could always select the lowest. For reads from NVME/SSD I would
normally expect something simple: speed_factor * (pending_bytes + req_bytes).
For HDDs this could do more computation like looking into what is there
in the queue already.

This would deviate from simple round-robin scheme, though.

Best Regards
Michał Mirosław

Re: [PATCH RFC] input/elants_i2c: Detect enum overflow

[Michał Mirosław]

On Wed, Feb 10, 2021 at 10:25:28AM -0600, Josh Poimboeuf wrote:
> If an enum value were to get added without updating this switch
> statement, the unreachable() annotation would trigger undefined
> behavior, causing execution to fall through the end of the function,
> into the next one.
> 
> Make the error handling more robust for an unexpected enum value, by
> doing BUG() instead of unreachable().
> 
> Fixes the following objtool warning:
> 
>   drivers/input/touchscreen/elants_i2c.o: warning: objtool: 
> elants_i2c_initialize() falls through to next function elants_i2c_resume()
> 
> Reported-by: Randy Dunlap 
> Acked-by: Randy Dunlap 
> Signed-off-by: Josh Poimboeuf 

Reviewed-by: Michał Mirosław 

> ---
>  drivers/input/touchscreen/elants_i2c.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/elants_i2c.c 
> b/drivers/input/touchscreen/elants_i2c.c
> index 6f57ec579f00..4c2b579f6c8b 100644
> --- a/drivers/input/touchscreen/elants_i2c.c
> +++ b/drivers/input/touchscreen/elants_i2c.c
> @@ -656,8 +656,7 @@ static int elants_i2c_initialize(struct elants_data *ts)
>   error = elants_i2c_query_ts_info_ektf(ts);
>   break;
>   default:
> - unreachable();
> - break;
> + BUG();
>   }
>  
>   if (error)
> -- 
> 2.29.2
> 

Re: [PATCH RFC 6/6] btrfs: Add roundrobin raid1 read policy

[Michał Mirosław]

On Wed, Feb 10, 2021 at 12:29:25PM +, Michal Rostecki wrote:
> On Wed, Feb 10, 2021 at 05:24:28AM +0100, Michał Mirosław wrote:
> > On Tue, Feb 09, 2021 at 09:30:40PM +0100, Michal Rostecki wrote:
> > [...]
> > > For the array with 3 HDDs, not adding any penalty resulted in 409MiB/s
> > > (429MB/s) performance. Adding the penalty value 1 resulted in a
> > > performance drop to 404MiB/s (424MB/s). Increasing the value towards 10
> > > was making the performance even worse.
> > > 
> > > For the array with 2 HDDs and 1 SSD, adding penalty value 1 to
> > > rotational disks resulted in the best performance - 541MiB/s (567MB/s).
> > > Not adding any value and increasing the value was making the performance
> > > worse.
> > > 
> > > Adding penalty value to non-rotational disks was always decreasing the
> > > performance, which motivated setting it as 0 by default. For the purpose
> > > of testing, it's still configurable.
> > [...]
> > > + bdev = map->stripes[mirror_index].dev->bdev;
> > > + inflight = mirror_load(fs_info, map, mirror_index, stripe_offset,
> > > +stripe_nr);
> > > + queue_depth = blk_queue_depth(bdev->bd_disk->queue);
> > > +
> > > + return inflight < queue_depth;
> > [...]
> > > + last_mirror = this_cpu_read(*fs_info->last_mirror);
> > [...]
> > > + for (i = last_mirror; i < first + num_stripes; i++) {
> > > + if (mirror_queue_not_filled(fs_info, map, i, stripe_offset,
> > > + stripe_nr)) {
> > > + preferred_mirror = i;
> > > + goto out;
> > > + }
> > > + }
> > > +
> > > + for (i = first; i < last_mirror; i++) {
> > > + if (mirror_queue_not_filled(fs_info, map, i, stripe_offset,
> > > + stripe_nr)) {
> > > + preferred_mirror = i;
> > > + goto out;
> > > + }
> > > + }
> > > +
> > > + preferred_mirror = last_mirror;
> > > +
> > > +out:
> > > + this_cpu_write(*fs_info->last_mirror, preferred_mirror);
> > 
> > This looks like it effectively decreases queue depth for non-last
> > device. After all devices are filled to queue_depth-penalty, only
> > a single mirror will be selected for next reads (until a read on
> > some other one completes).
> > 
> 
> Good point. And if all devices are going to be filled for longer time,
> this function will keep selecting the last one. Maybe I should select
> last+1 in that case. Would that address your concern or did you have any
> other solution in mind?

The best would be to postpone the selection until one device becomes free
again. But if that's not doable, then yes, you could make sure it stays
round-robin after filling the queues (the scheduling will loose the
"penalty"-driven adjustment though).

Best Reagrds,
Michał Mirosław

Re: [PATCH RFC 6/6] btrfs: Add roundrobin raid1 read policy

[Michał Mirosław]

On Tue, Feb 09, 2021 at 09:30:40PM +0100, Michal Rostecki wrote:
[...]
> For the array with 3 HDDs, not adding any penalty resulted in 409MiB/s
> (429MB/s) performance. Adding the penalty value 1 resulted in a
> performance drop to 404MiB/s (424MB/s). Increasing the value towards 10
> was making the performance even worse.
> 
> For the array with 2 HDDs and 1 SSD, adding penalty value 1 to
> rotational disks resulted in the best performance - 541MiB/s (567MB/s).
> Not adding any value and increasing the value was making the performance
> worse.
> 
> Adding penalty value to non-rotational disks was always decreasing the
> performance, which motivated setting it as 0 by default. For the purpose
> of testing, it's still configurable.
[...]
> + bdev = map->stripes[mirror_index].dev->bdev;
> + inflight = mirror_load(fs_info, map, mirror_index, stripe_offset,
> +stripe_nr);
> + queue_depth = blk_queue_depth(bdev->bd_disk->queue);
> +
> + return inflight < queue_depth;
[...]
> + last_mirror = this_cpu_read(*fs_info->last_mirror);
[...]
> + for (i = last_mirror; i < first + num_stripes; i++) {
> + if (mirror_queue_not_filled(fs_info, map, i, stripe_offset,
> + stripe_nr)) {
> + preferred_mirror = i;
> + goto out;
> + }
> + }
> +
> + for (i = first; i < last_mirror; i++) {
> + if (mirror_queue_not_filled(fs_info, map, i, stripe_offset,
> + stripe_nr)) {
> + preferred_mirror = i;
> + goto out;
> + }
> + }
> +
> + preferred_mirror = last_mirror;
> +
> +out:
> + this_cpu_write(*fs_info->last_mirror, preferred_mirror);

This looks like it effectively decreases queue depth for non-last
device. After all devices are filled to queue_depth-penalty, only
a single mirror will be selected for next reads (until a read on
some other one completes).

Have you tried testing with much more jobs / non-sequential accesses?

Best Reagrds,
Michał Mirosław

Re: [PATCH RFC 4/6] btrfs: Check if the filesystem is has mixed type of devices

[Michał Mirosław]

On Tue, Feb 09, 2021 at 09:30:38PM +0100, Michal Rostecki wrote:
> From: Michal Rostecki 
> 
> Add the btrfs_check_mixed() function which checks if the filesystem has
> the mixed type of devices (non-rotational and rotational). This
> information is going to be used in roundrobin raid1 read policy.a
[...]
> @@ -669,8 +699,12 @@ static int btrfs_open_one_device(struct btrfs_fs_devices 
> *fs_devices,
>   }
>  
>   q = bdev_get_queue(bdev);
> - if (!blk_queue_nonrot(q))
> + rotating = !blk_queue_nonrot(q);
> + device->rotating = rotating;
> + if (rotating)
>   fs_devices->rotating = true;
> + if (!fs_devices->mixed)
> + fs_devices->mixed = btrfs_check_mixed(fs_devices, rotating);
[...]

Since this is adding to a set, a faster way is:

if (fs_devices->rotating != rotating)
fs_devices->mixed = true;

The scan might be necessary on device removal, though.

> - if (!blk_queue_nonrot(q))
> + rotating = !blk_queue_nonrot(q);
> + device->rotating = rotating;
> + if (rotating)
>   fs_devices->rotating = true;
> + if (!fs_devices->mixed)
> + fs_devices->mixed = btrfs_check_mixed(fs_devices, rotating);

Duplication. Maybe pull all this into a function?

Best Regards,
Michał Mirosław

Re: [PATCH] mmc: cb710: Use new tasklet API

[Michał Mirosław]

On Mon, Feb 08, 2021 at 02:45:51PM +0100, Emil Renner Berthing wrote:
> This converts the driver to use the new tasklet API introduced in
> commit 12cc923f1ccc ("tasklet: Introduce new initialization API")
> 
> Signed-off-by: Emil Renner Berthing 

Acked-by: Michał Mirosław 

> ---
>  drivers/mmc/host/cb710-mmc.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mmc/host/cb710-mmc.c b/drivers/mmc/host/cb710-mmc.c
> index e84ed84ea4cc..6d623b2681c3 100644
> --- a/drivers/mmc/host/cb710-mmc.c
> +++ b/drivers/mmc/host/cb710-mmc.c
> @@ -646,14 +646,14 @@ static int cb710_mmc_irq_handler(struct cb710_slot 
> *slot)
>   return 1;
>  }
>  
> -static void cb710_mmc_finish_request_tasklet(unsigned long data)
> +static void cb710_mmc_finish_request_tasklet(struct tasklet_struct *t)
>  {
> - struct mmc_host *mmc = (void *)data;
> - struct cb710_mmc_reader *reader = mmc_priv(mmc);
> + struct cb710_mmc_reader *reader = from_tasklet(reader, t,
> +finish_req_tasklet);
>   struct mmc_request *mrq = reader->mrq;
>  
>   reader->mrq = NULL;
> - mmc_request_done(mmc, mrq);
> + mmc_request_done(mmc_from_priv(reader), mrq);
>  }
>  
>  static const struct mmc_host_ops cb710_mmc_host = {
> @@ -718,8 +718,8 @@ static int cb710_mmc_init(struct platform_device *pdev)
>  
>   reader = mmc_priv(mmc);
>  
> - tasklet_init(>finish_req_tasklet,
> - cb710_mmc_finish_request_tasklet, (unsigned long)mmc);
> + tasklet_setup(>finish_req_tasklet,
> +   cb710_mmc_finish_request_tasklet);
>   spin_lock_init(>irq_lock);
>   cb710_dump_regs(chip, CB710_DUMP_REGS_MMC);
>  

Re: [PATCH RESEND v8 2/4] input: elants: support old touch report format

[Michał Mirosław]

On Fri, Jan 22, 2021 at 11:10:52PM +0300, Dmitry Osipenko wrote:
> 08.01.2021 01:06, Dmitry Osipenko пишет:
> > 11.12.2020 21:48, Dmitry Torokhov пишет:
> >> On Fri, Dec 11, 2020 at 06:04:01PM +0100, Michał Mirosław wrote:
> >>> On Fri, Dec 11, 2020 at 07:39:33PM +0300, Dmitry Osipenko wrote:
> >>>> 11.12.2020 19:09, Michał Mirosław пишет:
> >>>>> On Thu, Dec 10, 2020 at 11:29:40PM -0800, Dmitry Torokhov wrote:
> >>>>>> Hi Michał,
> >>>>>> On Fri, Dec 11, 2020 at 07:53:56AM +0100, Michał Mirosław wrote:
> >>>>>>> @@ -998,17 +1011,18 @@ static irqreturn_t elants_i2c_irq(int irq, 
> >>>>>>> void *_dev)
> >>>>>>>   }
> >>>>>>>  
> >>>>>>>   report_len = ts->buf[FW_HDR_LENGTH] / 
> >>>>>>> report_count;
> >>>>>>> - if (report_len != PACKET_SIZE) {
> >>>>>>> + if (report_len != PACKET_SIZE &&
> >>>>>>> + report_len != PACKET_SIZE_OLD) {
> >>>>>>>   dev_err(>dev,
> >>>>>>> - "mismatching report length: 
> >>>>>>> %*ph\n",
> >>>>>>> + "unsupported report length: 
> >>>>>>> %*ph\n",
> >>>>>>>   HEADER_SIZE, ts->buf);
> >>>>>> Do I understand this correctly that the old packets are only observed 
> >>>>>> on
> >>>>>> EKTF3624? If so can we expand the check so that we only accept packets
> >>>>>> with "old" size when we know we are dealing with this device?
> >>>>>
> >>>>> We only have EKTF3624 and can't be sure there are no other chips 
> >>>>> needing this.
> >>>>
> >>>> In practice this older packet format should be seen only on 3624, but
> >>>> nevertheless we could make it more explicit by adding the extra chip_id
> >>>> checks.
> >>>>
> >>>> It won't be difficult to change it in the future if will be needed.
> >>>>
> >>>> I think the main point that Dmitry Torokhov conveys here is that we
> >>>> should minimize the possible impact on the current EKT3500 code since we
> >>>> don't have definitive answers regarding the firmware differences among
> >>>> the hardware variants.
> >>>
> >>> The only possible impact here is that older firmware instead of breaking
> >>> would suddenly work. Maybe we can accept such a risk?
> >>
> >> These are not controllers we'll randomly find in devices: Windows boxes
> >> use I2C HID, Chrome devices use "new" firmware, so that leaves random
> >> ARM where someone needs to consciously add proper compatible before the
> >> driver will engage with the controller.
> >>
> >> I would prefer we were conservative and not accept potentially invalid
> >> data.
> >>
> >> Thanks.
> >>
> > 
> > Michał, will you be able to make v9 with all the review comments addressed?
> > 
> 
> I'll make a v9 over this weekend.
> 
> Michał, please let me know if you already started to work on this or
> have any objections.

Hi,

Sorry for staying quiet so long. I have to revive my Transformer before
I can test anything, so please go ahead.

Best Regards
Michał Mirosław

Re: [PATCH v1 1/2] net: phy: Add 100 base-x mode

[Michał Mirosław]

pon., 11 sty 2021 o 14:54 Bjarni Jonasson
 napisał(a):
> Sparx-5 supports this mode and it is missing in the PHY core.
>
> Signed-off-by: Bjarni Jonasson 
> ---
>  include/linux/phy.h | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 56563e5e0dc7..dce867222d58 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -111,6 +111,7 @@ extern const int phy_10gbit_features_array[1];
>   * @PHY_INTERFACE_MODE_10GBASER: 10G BaseR
>   * @PHY_INTERFACE_MODE_USXGMII:  Universal Serial 10GE MII
>   * @PHY_INTERFACE_MODE_10GKR: 10GBASE-KR - with Clause 73 AN
> + * @PHY_INTERFACE_MODE_100BASEX: 100 BaseX
>   * @PHY_INTERFACE_MODE_MAX: Book keeping
[...]

This is kernel-internal interface, so maybe the new mode can be
inserted before 1000baseX for easier lookup?

Best Regards
Michał Mirosław

Re: [f2fs-dev] [PATCH v7 0/3] Update to zstd-1.4.6

[Michał Mirosław]

On Wed, Dec 16, 2020 at 10:07:38PM +, Nick Terrell wrote:
[...]
> It is very large. If it helps, in the commit message I’ve provided this link 
> [0],
> which provides the diff between upstream zstd as-is and the imported zstd,
> which has been modified by the automated tooling to work in the kernel.
> [0] 
> https://github.com/terrelln/linux/commit/ac2ee65dcb7318afe426ad08f6a844faf3aebb41

I looks like you could remove a bit more dead code by noting __GNUC__ >= 4
(gcc-4.9 is currently the oldest supported [1]).

[1] Documentation/process/changes.rst

Best Regards
Michał Mirosław

Re: [PATCH RESEND v8 2/4] input: elants: support old touch report format

[Michał Mirosław]

On Fri, Dec 11, 2020 at 07:39:33PM +0300, Dmitry Osipenko wrote:
> 11.12.2020 19:09, Michał Mirosław пишет:
> > On Thu, Dec 10, 2020 at 11:29:40PM -0800, Dmitry Torokhov wrote:
> >> Hi Michał,
> >> On Fri, Dec 11, 2020 at 07:53:56AM +0100, Michał Mirosław wrote:
> >>> @@ -998,17 +1011,18 @@ static irqreturn_t elants_i2c_irq(int irq, void 
> >>> *_dev)
> >>>   }
> >>>  
> >>>   report_len = ts->buf[FW_HDR_LENGTH] / report_count;
> >>> - if (report_len != PACKET_SIZE) {
> >>> + if (report_len != PACKET_SIZE &&
> >>> + report_len != PACKET_SIZE_OLD) {
> >>>   dev_err(>dev,
> >>> - "mismatching report length: %*ph\n",
> >>> + "unsupported report length: %*ph\n",
> >>>   HEADER_SIZE, ts->buf);
> >> Do I understand this correctly that the old packets are only observed on
> >> EKTF3624? If so can we expand the check so that we only accept packets
> >> with "old" size when we know we are dealing with this device?
> > 
> > We only have EKTF3624 and can't be sure there are no other chips needing 
> > this.
> 
> In practice this older packet format should be seen only on 3624, but
> nevertheless we could make it more explicit by adding the extra chip_id
> checks.
> 
> It won't be difficult to change it in the future if will be needed.
> 
> I think the main point that Dmitry Torokhov conveys here is that we
> should minimize the possible impact on the current EKT3500 code since we
> don't have definitive answers regarding the firmware differences among
> the hardware variants.

The only possible impact here is that older firmware instead of breaking
would suddenly work. Maybe we can accept such a risk?

Best Regards
Michał Mirosław

Re: [PATCH RESEND v8 3/4] input: elants: read touchscreen size for EKTF3624

[Michał Mirosław]

On Thu, Dec 10, 2020 at 11:22:09PM -0800, Dmitry Torokhov wrote:
> Hi Michał,
> 
> On Fri, Dec 11, 2020 at 07:53:56AM +0100, Michał Mirosław wrote:
> > +
> > +   if (!phy_x || !phy_y) {
> > +   dev_warn(>dev,
> > +"invalid size data: %d x %d mm\n",
> > +phy_x, phy_y);
> > +   return 0;
> 
> Given we are not treating this as hard error mind dropping this "return"
> and making the below an "else" branch?

It is an error, still, and saves a bit of indentation in the following
normal-path code. But I see that there is already a similar code with
an 'else' branch.

Best Regards,
Michał Mirosław

Re: [PATCH RESEND v8 2/4] input: elants: support old touch report format

[Michał Mirosław]

On Thu, Dec 10, 2020 at 11:29:40PM -0800, Dmitry Torokhov wrote:
> Hi Michał,
> On Fri, Dec 11, 2020 at 07:53:56AM +0100, Michał Mirosław wrote:
> > @@ -998,17 +1011,18 @@ static irqreturn_t elants_i2c_irq(int irq, void 
> > *_dev)
> > }
> >  
> > report_len = ts->buf[FW_HDR_LENGTH] / report_count;
> > -   if (report_len != PACKET_SIZE) {
> > +   if (report_len != PACKET_SIZE &&
> > +   report_len != PACKET_SIZE_OLD) {
> > dev_err(>dev,
> > -   "mismatching report length: %*ph\n",
> > +   "unsupported report length: %*ph\n",
> > HEADER_SIZE, ts->buf);
> Do I understand this correctly that the old packets are only observed on
> EKTF3624? If so can we expand the check so that we only accept packets
> with "old" size when we know we are dealing with this device?

We only have EKTF3624 and can't be sure there are no other chips needing this.

Best Regards
Michał Mirosław

[PATCH RESEND v8 1/4] input: elants: document some registers and values

[Michał Mirosław]

Add information found in downstream kernels, to make the code less
magic.

Signed-off-by: Michał Mirosław 
Reviewed-by: Dmitry Osipenko 
Tested-by: Dmitry Osipenko 
---
 drivers/input/touchscreen/elants_i2c.c | 18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/elants_i2c.c 
b/drivers/input/touchscreen/elants_i2c.c
index 50c348297e38..d51cb910fba1 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -82,7 +82,7 @@
 
 #define HEADER_REPORT_10_FINGER0x62
 
-/* Header (4 bytes) plus 3 fill 10-finger packets */
+/* Header (4 bytes) plus 3 full 10-finger packets */
 #define MAX_PACKET_SIZE169
 
 #define BOOT_TIME_DELAY_MS 50
@@ -97,6 +97,10 @@
 #define E_INFO_PHY_SCAN0xD7
 #define E_INFO_PHY_DRIVER  0xD8
 
+/* FW write command, 0x54 0x?? 0x0, 0x01 */
+#define E_POWER_STATE_SLEEP0x50
+#define E_POWER_STATE_RESUME   0x58
+
 #define MAX_RETRIES3
 #define MAX_FW_UPDATE_RETRIES  30
 
@@ -269,8 +273,8 @@ static int elants_i2c_calibrate(struct elants_data *ts)
 {
struct i2c_client *client = ts->client;
int ret, error;
-   static const u8 w_flashkey[] = { 0x54, 0xC0, 0xE1, 0x5A };
-   static const u8 rek[] = { 0x54, 0x29, 0x00, 0x01 };
+   static const u8 w_flashkey[] = { CMD_HEADER_WRITE, 0xC0, 0xE1, 0x5A };
+   static const u8 rek[] = { CMD_HEADER_WRITE, 0x29, 0x00, 0x01 };
static const u8 rek_resp[] = { CMD_HEADER_REK, 0x66, 0x66, 0x66 };
 
disable_irq(client->irq);
@@ -1388,7 +1392,9 @@ static int __maybe_unused elants_i2c_suspend(struct 
device *dev)
 {
struct i2c_client *client = to_i2c_client(dev);
struct elants_data *ts = i2c_get_clientdata(client);
-   const u8 set_sleep_cmd[] = { 0x54, 0x50, 0x00, 0x01 };
+   const u8 set_sleep_cmd[] = {
+   CMD_HEADER_WRITE, E_POWER_STATE_SLEEP, 0x00, 0x01
+   };
int retry_cnt;
int error;
 
@@ -1425,7 +1431,9 @@ static int __maybe_unused elants_i2c_resume(struct device 
*dev)
 {
struct i2c_client *client = to_i2c_client(dev);
struct elants_data *ts = i2c_get_clientdata(client);
-   const u8 set_active_cmd[] = { 0x54, 0x58, 0x00, 0x01 };
+   const u8 set_active_cmd[] = {
+   CMD_HEADER_WRITE, E_POWER_STATE_RESUME, 0x00, 0x01
+   };
int retry_cnt;
int error;
 
-- 
2.20.1

[PATCH RESEND v8 4/4] input: elants: support 0x66 reply opcode for reporting touches

[Michał Mirosław]

From: Dmitry Osipenko 

eKTF3624 touchscreen firmware uses two variants of the reply opcodes for
reporting touch events: one is 0x63 (used by older firmware) and other is
0x66 (used by newer firmware). The 0x66 variant is equal to 0x63 of
eKTH3500, while 0x63 needs small adjustment of the touch pressure value.

Nexus 7 tablet device has eKTF3624 touchscreen and it uses 0x66 opcode for
reporting touch events, let's support it now. Other devices, eg. ASUS TF300T,
use 0x63.

Note: CMD_HEADER_REK is used for replying to calibration requests, it has
the same 0x66 opcode number which eKTF3624 uses for reporting touches.
The calibration replies are handled separately from the the rest of the
commands in the driver by entering into ELAN_WAIT_RECALIBRATION state
and thus this change shouldn't change the old behavior.

Signed-off-by: Dmitry Osipenko 
Tested-by: Michał Mirosław 
Signed-off-by: Michał Mirosław 
---
 drivers/input/touchscreen/elants_i2c.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/elants_i2c.c 
b/drivers/input/touchscreen/elants_i2c.c
index c24d8cdc4251..1cbda6f20d07 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -61,6 +61,15 @@
 #define QUEUE_HEADER_NORMAL0X63
 #define QUEUE_HEADER_WAIT  0x64
 
+/*
+ * Depending on firmware version, eKTF3624 touchscreens may utilize one of
+ * these opcodes for the touch events: 0x63 and 0x66. The 0x63 is used by
+ * older firmware version and differs from 0x66 such that touch pressure
+ * value needs to be adjusted. The 0x66 opcode of newer firmware is equal
+ * to 0x63 of eKTH3500.
+ */
+#define QUEUE_HEADER_NORMAL2   0x66
+
 /* Command header definition */
 #define CMD_HEADER_WRITE   0x54
 #define CMD_HEADER_READ0x53
@@ -1052,7 +1061,6 @@ static irqreturn_t elants_i2c_irq(int irq, void *_dev)
switch (ts->buf[FW_HDR_TYPE]) {
case CMD_HEADER_HELLO:
case CMD_HEADER_RESP:
-   case CMD_HEADER_REK:
break;
 
case QUEUE_HEADER_WAIT:
@@ -1072,6 +1080,7 @@ static irqreturn_t elants_i2c_irq(int irq, void *_dev)
break;
 
case QUEUE_HEADER_NORMAL:
+   case QUEUE_HEADER_NORMAL2:
report_count = ts->buf[FW_HDR_COUNT];
if (report_count == 0 || report_count > 3) {
dev_err(>dev,
-- 
2.20.1

[PATCH RESEND v8 3/4] input: elants: read touchscreen size for EKTF3624

[Michał Mirosław]

EKTF3624 as present in Asus TF300T tablet has touchscreen size encoded
in different registers.

Signed-off-by: Michał Mirosław 
Reviewed-by: Dmitry Osipenko 
Tested-by: Dmitry Osipenko 
---
 drivers/input/touchscreen/elants_i2c.c | 84 --
 1 file changed, 79 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/elants_i2c.c 
b/drivers/input/touchscreen/elants_i2c.c
index f1bf3e000e96..c24d8cdc4251 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -35,7 +35,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
@@ -43,6 +43,10 @@
 /* Device, Driver information */
 #define DEVICE_NAME"elants_i2c"
 
+/* Device IDs */
+#define EKTH3500   0
+#define EKTF3624   1
+
 /* Convert from rows or columns into resolution */
 #define ELAN_TS_RESOLUTION(n, m)   (((n) - 1) * (m))
 
@@ -94,6 +98,8 @@
 #define E_ELAN_INFO_REK0xD0
 #define E_ELAN_INFO_TEST_VER   0xE0
 #define E_ELAN_INFO_FW_ID  0xF0
+#define E_INFO_X_RES   0x60
+#define E_INFO_Y_RES   0x63
 #define E_INFO_OSR 0xD6
 #define E_INFO_PHY_SCAN0xD7
 #define E_INFO_PHY_DRIVER  0xD8
@@ -157,6 +163,7 @@ struct elants_data {
 
bool wake_irq_enabled;
bool keep_power_in_suspend;
+   u8 chip_id;
 
/* Must be last to be used for DMA operations */
u8 buf[MAX_PACKET_SIZE] cacheline_aligned;
@@ -434,7 +441,58 @@ static int elants_i2c_query_bc_version(struct elants_data 
*ts)
return 0;
 }
 
-static int elants_i2c_query_ts_info(struct elants_data *ts)
+static int elants_i2c_query_ts_info_ektf(struct elants_data *ts)
+{
+   struct i2c_client *client = ts->client;
+   int error;
+   u8 resp[4];
+   u16 phy_x, phy_y;
+   const u8 get_xres_cmd[] = {
+   CMD_HEADER_READ, E_INFO_X_RES, 0x00, 0x00
+   };
+   const u8 get_yres_cmd[] = {
+   CMD_HEADER_READ, E_INFO_Y_RES, 0x00, 0x00
+   };
+
+   /* Get X/Y size in mm */
+   error = elants_i2c_execute_command(client, get_xres_cmd,
+  sizeof(get_xres_cmd),
+  resp, sizeof(resp), 1,
+  "get X size");
+   if (error)
+   return error;
+
+   phy_x = resp[2] | ((resp[3] & 0xF0) << 4);
+
+   error = elants_i2c_execute_command(client, get_yres_cmd,
+  sizeof(get_yres_cmd),
+  resp, sizeof(resp), 1,
+  "get Y size");
+   if (error)
+   return error;
+
+   phy_y = resp[2] | ((resp[3] & 0xF0) << 4);
+
+   if (!phy_x || !phy_y) {
+   dev_warn(>dev,
+"invalid size data: %d x %d mm\n",
+phy_x, phy_y);
+   return 0;
+   }
+
+   dev_dbg(>dev, "phy_x=%d, phy_y=%d\n", phy_x, phy_y);
+
+   /* calculate resolution from size */
+   ts->x_max = 2240-1;
+   ts->x_res = DIV_ROUND_CLOSEST(ts->prop.max_x, phy_x);
+
+   ts->y_max = 1408-1;
+   ts->y_res = DIV_ROUND_CLOSEST(ts->prop.max_y, phy_y);
+
+   return 0;
+}
+
+static int elants_i2c_query_ts_info_ekth(struct elants_data *ts)
 {
struct i2c_client *client = ts->client;
int error;
@@ -588,8 +646,20 @@ static int elants_i2c_initialize(struct elants_data *ts)
error = elants_i2c_query_fw_version(ts);
if (!error)
error = elants_i2c_query_test_version(ts);
-   if (!error)
-   error = elants_i2c_query_ts_info(ts);
+
+   switch (ts->chip_id) {
+   case EKTH3500:
+   if (!error)
+   error = elants_i2c_query_ts_info_ekth(ts);
+   break;
+   case EKTF3624:
+   if (!error)
+   error = elants_i2c_query_ts_info_ektf(ts);
+   break;
+   default:
+   unreachable();
+   break;
+   }
 
if (error)
ts->iap_mode = ELAN_IAP_RECOVERY;
@@ -1266,6 +1336,9 @@ static int elants_i2c_probe(struct i2c_client *client,
ts->client = client;
i2c_set_clientdata(client, ts);
 
+   if (client->dev.of_node)
+   ts->chip_id = (uintptr_t)of_device_get_match_data(>dev);
+
ts->vcc33 = devm_regulator_get(>dev, "vcc33");
if (IS_ERR(ts->vcc33)) {
error = PTR_ERR(ts->vcc33);
@@ -1495,7 +1568,8 @@ MODULE_DEVICE_TABLE(acpi, elants_acpi_id);
 
 #ifdef CONFIG_OF
 static const struct of_device_id elants_of_match[] = {
-   { .compatible = "elan,ekth3500" },
+   { .compatible = "elan,ekth3500", .data = 

[PATCH RESEND v8 2/4] input: elants: support old touch report format

[Michał Mirosław]

Support ELAN touchpad sensor with older firmware as found on eg. Asus
Transformer Pads.

Signed-off-by: Michał Mirosław 
Reviewed-by: Dmitry Osipenko 
Tested-by: Dmitry Osipenko 
---
 drivers/input/touchscreen/elants_i2c.c | 36 ++
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/input/touchscreen/elants_i2c.c 
b/drivers/input/touchscreen/elants_i2c.c
index d51cb910fba1..f1bf3e000e96 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -69,6 +69,7 @@
 #define CMD_HEADER_REK 0x66
 
 /* FW position data */
+#define PACKET_SIZE_OLD40
 #define PACKET_SIZE55
 #define MAX_CONTACT_NUM10
 #define FW_POS_HEADER  0
@@ -853,7 +854,8 @@ static int elants_i2c_fw_update(struct elants_data *ts)
  * Event reporting.
  */
 
-static void elants_i2c_mt_event(struct elants_data *ts, u8 *buf)
+static void elants_i2c_mt_event(struct elants_data *ts, u8 *buf,
+   size_t report_len)
 {
struct input_dev *input = ts->input;
unsigned int n_fingers;
@@ -866,7 +868,8 @@ static void elants_i2c_mt_event(struct elants_data *ts, u8 
*buf)
buf[FW_POS_STATE];
 
dev_dbg(>client->dev,
-   "n_fingers: %u, state: %04x\n",  n_fingers, finger_state);
+   "n_fingers: %u, state: %04x, report_len: %zu\n",
+   n_fingers, finger_state, report_len);
 
/* Note: all fingers have the same tool type */
tool_type = buf[FW_POS_TOOL_TYPE] & BIT(0) ?
@@ -880,8 +883,16 @@ static void elants_i2c_mt_event(struct elants_data *ts, u8 
*buf)
pos = [FW_POS_XY + i * 3];
x = (((u16)pos[0] & 0xf0) << 4) | pos[1];
y = (((u16)pos[0] & 0x0f) << 8) | pos[2];
-   p = buf[FW_POS_PRESSURE + i];
-   w = buf[FW_POS_WIDTH + i];
+   if (report_len == PACKET_SIZE_OLD) {
+   w = buf[FW_POS_WIDTH + i / 2];
+   w >>= 4 * (~i & 1); // little-endian-nibbles
+   w |= w << 4;
+   w |= !w;
+   p = w;
+   } else {
+   p = buf[FW_POS_PRESSURE + i];
+   w = buf[FW_POS_WIDTH + i];
+   }
 
dev_dbg(>client->dev, "i=%d x=%d y=%d p=%d w=%d\n",
i, x, y, p, w);
@@ -913,7 +924,8 @@ static u8 elants_i2c_calculate_checksum(u8 *buf)
return checksum;
 }
 
-static void elants_i2c_event(struct elants_data *ts, u8 *buf)
+static void elants_i2c_event(struct elants_data *ts, u8 *buf,
+size_t report_len)
 {
u8 checksum = elants_i2c_calculate_checksum(buf);
 
@@ -927,7 +939,7 @@ static void elants_i2c_event(struct elants_data *ts, u8 
*buf)
 "%s: unknown packet type: %02x\n",
 __func__, buf[FW_POS_HEADER]);
else
-   elants_i2c_mt_event(ts, buf);
+   elants_i2c_mt_event(ts, buf, report_len);
 }
 
 static irqreturn_t elants_i2c_irq(int irq, void *_dev)
@@ -985,7 +997,8 @@ static irqreturn_t elants_i2c_irq(int irq, void *_dev)
break;
 
case QUEUE_HEADER_SINGLE:
-   elants_i2c_event(ts, >buf[HEADER_SIZE]);
+   elants_i2c_event(ts, >buf[HEADER_SIZE],
+ts->buf[FW_HDR_LENGTH]);
break;
 
case QUEUE_HEADER_NORMAL:
@@ -998,17 +1011,18 @@ static irqreturn_t elants_i2c_irq(int irq, void *_dev)
}
 
report_len = ts->buf[FW_HDR_LENGTH] / report_count;
-   if (report_len != PACKET_SIZE) {
+   if (report_len != PACKET_SIZE &&
+   report_len != PACKET_SIZE_OLD) {
dev_err(>dev,
-   "mismatching report length: %*ph\n",
+   "unsupported report length: %*ph\n",
HEADER_SIZE, ts->buf);
break;
}
 
for (i = 0; i < report_count; i++) {
u8 *buf = ts->buf + HEADER_SIZE +
-   i * PACKET_SIZE;
-   elants_i2c_event(ts, buf);
+ i * report_len;
+   elants_i2c_event(ts, buf, report_len);
}
break;
 
-- 
2.20.1

[PATCH RESEND v8 0/4] input: elants: Support Asus TF300T and Nexus 7 touchscreens

[Michał Mirosław]

This series cleans up the driver a bit and implements changes needed to
support EKTF3624-based touchscreen used in Asus TF300T, Google Nexus 7
and similar Tegra3-based tablets.

---
v2: extended with Dmitry's patches (replaced v1 patches 3 and 4)
v3: rebased for v5.7-rc1
v4: rebased onto v5.7-rc2+ (current Linus' master)
update "remove unused axes" and "refactor
  elants_i2c_execute_command()" patches after review
add David's patch converting DT binding to YAML
v5: rebased onto dtor/input/for-linus
v6: rebased onto newer dtor/input/for-linus
remove yet unused constants from patch 1
added a new drive-by cleanup (last patch)
v7: rebased onto current dtor/input/for-next
v8: rebased onto current dtor/input/for-linus
v8-resend: rebased again, no changes though
---

Dmitry Osipenko (1):
  input: elants: support 0x66 reply opcode for reporting touches

Michał Mirosław (3):
  input: elants: document some registers and values
  input: elants: support old touch report format
  input: elants: read touchscreen size for EKTF3624

 drivers/input/touchscreen/elants_i2c.c | 149 +
 1 file changed, 127 insertions(+), 22 deletions(-)

-- 
2.20.1

Re: [PATCH v6 1/3] lib: zstd: Add kernel-specific API

[Michał Mirosław]

On Thu, Dec 03, 2020 at 03:59:21AM +, Nick Terrell wrote:
> On Dec 2, 2020, at 7:14 PM, Michał Mirosław  wrote:
> > On Thu, Dec 03, 2020 at 01:42:03AM +, Nick Terrell wrote:
> >> On Dec 2, 2020, at 5:16 PM, Michał Mirosław  
> >> wrote:
> >>> On Wed, Dec 02, 2020 at 12:32:40PM -0800, Nick Terrell wrote:
> >>>> From: Nick Terrell 
> >>>> 
> >>>> This patch:
> >>>> - Moves `include/linux/zstd.h` -> `lib/zstd/zstd.h`
> >>>> - Adds a new API in `include/linux/zstd.h` that is functionally
> >>>> equivalent to the in-use subset of the current API. Functions are
> >>>> renamed to avoid symbol collisions with zstd, to make it clear it is
> >>>> not the upstream zstd API, and to follow the kernel style guide.
> >>>> - Updates all callers to use the new API.
> >>>> 
> >>>> There are no functional changes in this patch. Since there are no
> >>>> functional change, I felt it was okay to update all the callers in a
> >>>> single patch, since once the API is approved, the callers are
> >>>> mechanically changed.
> >>> [...]
> >>>> --- a/lib/decompress_unzstd.c
> >>>> +++ b/lib/decompress_unzstd.c
> >>> [...]
> >>>> static int INIT handle_zstd_error(size_t ret, void (*error)(char *x))
> >>>> {
> >>>> -const int err = ZSTD_getErrorCode(ret);
> >>>> -
> >>>> -if (!ZSTD_isError(ret))
> >>>> +if (!zstd_is_error(ret))
> >>>>  return 0;
> >>>> 
> >>>> -switch (err) {
> >>>> -case ZSTD_error_memory_allocation:
> >>>> -error("ZSTD decompressor ran out of memory");
> >>>> -break;
> >>>> -case ZSTD_error_prefix_unknown:
> >>>> -error("Input is not in the ZSTD format (wrong magic 
> >>>> bytes)");
> >>>> -break;
> >>>> -case ZSTD_error_dstSize_tooSmall:
> >>>> -case ZSTD_error_corruption_detected:
> >>>> -case ZSTD_error_checksum_wrong:
> >>>> -error("ZSTD-compressed data is corrupt");
> >>>> -break;
> >>>> -default:
> >>>> -error("ZSTD-compressed data is probably corrupt");
> >>>> -break;
> >>>> -}
> >>>> +error("ZSTD decompression failed");
> >>>>  return -1;
> >>>> }
> >>> 
> >>> This looses diagnostics specificity - is this intended? At least the
> >>> out-of-memory condition seems useful to distinguish.
> >> 
> >> Good point. The zstd API no longer exposes the error code enum,
> >> but it does expose zstd_get_error_name() which can be used here.
> >> I was thinking that the string needed to be static for some reason, but
> >> that is not the case. I will make that change.
> >> 
> >>>> +size_t zstd_compress_stream(zstd_cstream *cstream,
> >>>> +struct zstd_out_buffer *output, struct zstd_in_buffer *input)
> >>>> +{
> >>>> +ZSTD_outBuffer o;
> >>>> +ZSTD_inBuffer i;
> >>>> +size_t ret;
> >>>> +
> >>>> +memcpy(, output, sizeof(o));
> >>>> +memcpy(, input, sizeof(i));
> >>>> +ret = ZSTD_compressStream(cstream, , );
> >>>> +memcpy(output, , sizeof(o));
> >>>> +memcpy(input, , sizeof(i));
> >>>> +return ret;
> >>>> +}
> >>> 
> >>> Is all this copying necessary? How is it different from type-punning by
> >>> direct pointer cast?
> >> 
> >> If breaking strict aliasing and type-punning by pointer casing is okay, 
> >> then
> >> we can do that here. These memcpys will be negligible for performance, but
> >> type-punning would be more succinct if allowed.
> > 
> > Ah, this might break LTO builds due to strict aliasing violation.
> > So I would suggest to just #define the ZSTD names to kernel ones
> > for the library code.  Unless there is a cleaner solution...
> 
> I don’t want to do that because I want in the 3rd series of the patchset I 
> update
> to zstd-1.4.6. And I’m using zstd-1.4.6 as-is in upstream. This allows us to 
> keep
> the kernel version up to date, since the patch to update to a new version can 
> be
> generated automatically (and manually tested), so it doesn’t fall years behind
> upstream again.
> 
> The alternative would be to make upstream zstd’s header public and
> #define zstd_in_buffer ZSTD_inBuffer. But that would make zstd’s header
> public, which would somewhat defeat the purpose of having a kernel wrapper.

I thought the problem was API style spill-over from the library to other parts
of the kernel.  A header-only wrapper can stop this.  I'm not sure symbol
visibility (namespace pollution) was a concern.

Best Regards
Michał Mirosław

Re: [PATCH v6 1/3] lib: zstd: Add kernel-specific API

[Michał Mirosław]

On Thu, Dec 03, 2020 at 01:42:03AM +, Nick Terrell wrote:
> 
> 
> > On Dec 2, 2020, at 5:16 PM, Michał Mirosław  wrote:
> > 
> > On Wed, Dec 02, 2020 at 12:32:40PM -0800, Nick Terrell wrote:
> >> From: Nick Terrell 
> >> 
> >> This patch:
> >> - Moves `include/linux/zstd.h` -> `lib/zstd/zstd.h`
> >> - Adds a new API in `include/linux/zstd.h` that is functionally
> >>  equivalent to the in-use subset of the current API. Functions are
> >>  renamed to avoid symbol collisions with zstd, to make it clear it is
> >>  not the upstream zstd API, and to follow the kernel style guide.
> >> - Updates all callers to use the new API.
> >> 
> >> There are no functional changes in this patch. Since there are no
> >> functional change, I felt it was okay to update all the callers in a
> >> single patch, since once the API is approved, the callers are
> >> mechanically changed.
> > [...]
> >> --- a/lib/decompress_unzstd.c
> >> +++ b/lib/decompress_unzstd.c
> > [...]
> >> static int INIT handle_zstd_error(size_t ret, void (*error)(char *x))
> >> {
> >> -  const int err = ZSTD_getErrorCode(ret);
> >> -
> >> -  if (!ZSTD_isError(ret))
> >> +  if (!zstd_is_error(ret))
> >>return 0;
> >> 
> >> -  switch (err) {
> >> -  case ZSTD_error_memory_allocation:
> >> -  error("ZSTD decompressor ran out of memory");
> >> -  break;
> >> -  case ZSTD_error_prefix_unknown:
> >> -  error("Input is not in the ZSTD format (wrong magic bytes)");
> >> -  break;
> >> -  case ZSTD_error_dstSize_tooSmall:
> >> -  case ZSTD_error_corruption_detected:
> >> -  case ZSTD_error_checksum_wrong:
> >> -  error("ZSTD-compressed data is corrupt");
> >> -  break;
> >> -  default:
> >> -  error("ZSTD-compressed data is probably corrupt");
> >> -  break;
> >> -  }
> >> +  error("ZSTD decompression failed");
> >>return -1;
> >> }
> > 
> > This looses diagnostics specificity - is this intended? At least the
> > out-of-memory condition seems useful to distinguish.
> 
> Good point. The zstd API no longer exposes the error code enum,
> but it does expose zstd_get_error_name() which can be used here.
> I was thinking that the string needed to be static for some reason, but
> that is not the case. I will make that change.
> 
> >> +size_t zstd_compress_stream(zstd_cstream *cstream,
> >> +  struct zstd_out_buffer *output, struct zstd_in_buffer *input)
> >> +{
> >> +  ZSTD_outBuffer o;
> >> +  ZSTD_inBuffer i;
> >> +  size_t ret;
> >> +
> >> +  memcpy(, output, sizeof(o));
> >> +  memcpy(, input, sizeof(i));
> >> +  ret = ZSTD_compressStream(cstream, , );
> >> +  memcpy(output, , sizeof(o));
> >> +  memcpy(input, , sizeof(i));
> >> +  return ret;
> >> +}
> > 
> > Is all this copying necessary? How is it different from type-punning by
> > direct pointer cast?
> 
> If breaking strict aliasing and type-punning by pointer casing is okay, then
> we can do that here. These memcpys will be negligible for performance, but
> type-punning would be more succinct if allowed.

Ah, this might break LTO builds due to strict aliasing violation.
So I would suggest to just #define the ZSTD names to kernel ones
for the library code.  Unless there is a cleaner solution...

Best Regards
Michał Mirosław

Re: [PATCH v6 1/3] lib: zstd: Add kernel-specific API

[Michał Mirosław]

On Wed, Dec 02, 2020 at 12:32:40PM -0800, Nick Terrell wrote:
> From: Nick Terrell 
> 
> This patch:
> - Moves `include/linux/zstd.h` -> `lib/zstd/zstd.h`
> - Adds a new API in `include/linux/zstd.h` that is functionally
>   equivalent to the in-use subset of the current API. Functions are
>   renamed to avoid symbol collisions with zstd, to make it clear it is
>   not the upstream zstd API, and to follow the kernel style guide.
> - Updates all callers to use the new API.
> 
> There are no functional changes in this patch. Since there are no
> functional change, I felt it was okay to update all the callers in a
> single patch, since once the API is approved, the callers are
> mechanically changed.
[...]
> --- a/lib/decompress_unzstd.c
> +++ b/lib/decompress_unzstd.c
[...]
>  static int INIT handle_zstd_error(size_t ret, void (*error)(char *x))
>  {
> - const int err = ZSTD_getErrorCode(ret);
> -
> - if (!ZSTD_isError(ret))
> + if (!zstd_is_error(ret))
>   return 0;
>  
> - switch (err) {
> - case ZSTD_error_memory_allocation:
> - error("ZSTD decompressor ran out of memory");
> - break;
> - case ZSTD_error_prefix_unknown:
> - error("Input is not in the ZSTD format (wrong magic bytes)");
> - break;
> - case ZSTD_error_dstSize_tooSmall:
> - case ZSTD_error_corruption_detected:
> - case ZSTD_error_checksum_wrong:
> - error("ZSTD-compressed data is corrupt");
> - break;
> - default:
> - error("ZSTD-compressed data is probably corrupt");
> - break;
> - }
> + error("ZSTD decompression failed");
>   return -1;
>  }

This looses diagnostics specificity - is this intended? At least the
out-of-memory condition seems useful to distinguish.

> +size_t zstd_compress_stream(zstd_cstream *cstream,
> + struct zstd_out_buffer *output, struct zstd_in_buffer *input)
> +{
> + ZSTD_outBuffer o;
> + ZSTD_inBuffer i;
> + size_t ret;
> +
> + memcpy(, output, sizeof(o));
> + memcpy(, input, sizeof(i));
> + ret = ZSTD_compressStream(cstream, , );
> +     memcpy(output, , sizeof(o));
> + memcpy(input, , sizeof(i));
> + return ret;
> +}

Is all this copying necessary? How is it different from type-punning by
direct pointer cast?

Best Regards
Michał Mirosław

Re: [PATCH] power: bq25890: Use the correct range for IILIM register

[Michał Mirosław]

On Wed, Nov 25, 2020 at 04:48:05AM +0100, Sebastian Krzyszkowiak wrote:
> I've checked bq25890, bq25892, bq25895 and bq25896 datasheets and
> they all define IILIM to be between 100mA-3.25A with 50mA steps.

That's what DS says, indeed. 

Reviewed-by: Michał Mirosław 

Re: About regression caused by commit aea6cb99703e ("regulator: resolve supply after creating regulator")

[Michał Mirosław]

On Sun, Nov 22, 2020 at 03:43:33PM +0100, Jan Kiszka wrote:
> On 09.11.20 00:28, Qu Wenruo wrote:
> > On 2020/11/9 上午1:18, Michał Mirosław wrote:
> >> On Sun, Nov 08, 2020 at 03:35:33PM +0800, Qu Wenruo wrote:
[...]
> >>> It turns out that, commit aea6cb99703e ("regulator: resolve supply after
> >>> creating regulator") seems to be the cause.
[...]
> We are still missing some magic fix for stable trees: On the STM32MP15x,
> things are broken since 5.4.73 now. And 5.9.y is not booting as well on
> that board. Reverting the original commit make it boot again.
> 
> Linus master is fine, though, but I'm tired of bisecting. Any
> suggestions? Or is there something queued up already?

You might want to look at `git log --grep=aea6cb99703e` if you can't
wait for a stable backport.

Best Regards
Michał Mirosław

[PATCH RESEND 0/4] regulator: debugging and fixing supply deps

[Michał Mirosław]

It turns out that commit aea6cb99703e ("regulator: resolve supply
after creating regulator") exposed a number of issues in regulator
initialization and introduced a memory leak of its own. One uncovered
problem was already fixed by cf1ad559a20d ("regulator: defer probe when
trying to get voltage from unresolved supply"). This series fixes the
remaining ones and adds a two debugging aids to help in the future.

The final patch adds a workaround to preexisting problem occurring with
regulators that have the same name as its supply_name. This worked
before by accident, so might be worth backporting. The error message is
left on purpose so that these configurations can be detected and fixed.

(The first two patches are resends from Nov 5).

(Series resent because of wrong arm-kernel ML address.)

Michał Mirosław (4):
  regulator: fix memory leak with repeated set_machine_constraints()
  regulator: debug early supply resolving
  regulator: avoid resolve_supply() infinite recursion
  regulator: workaround self-referent regulators

 drivers/regulator/core.c | 40 
 1 file changed, 24 insertions(+), 16 deletions(-)

-- 
2.20.1

[PATCH RESEND 3/4] regulator: avoid resolve_supply() infinite recursion

[Michał Mirosław]

When a regulator's name equals its supply's name the
regulator_resolve_supply() recurses indefinitely. Add a check
so that debugging the problem is easier. The "fixed" commit
just exposed the problem.

Fixes: aea6cb99703e ("regulator: resolve supply after creating regulator")
Cc: sta...@vger.kernel.org
Reported-by: Ahmad Fatoum 
Signed-off-by: Michał Mirosław 
---
 drivers/regulator/core.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index ad36f03d7ee6..ab922ed273f3 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1841,6 +1841,12 @@ static int regulator_resolve_supply(struct regulator_dev 
*rdev)
}
}
 
+   if (r == rdev) {
+   dev_err(dev, "Supply for %s (%s) resolved to itself\n",
+   rdev->desc->name, rdev->supply_name);
+   return -EINVAL;
+   }
+
/*
 * If the supply's parent device is not the same as the
 * regulator's parent device, then ensure the parent device
-- 
2.20.1

[PATCH RESEND 4/4] regulator: workaround self-referent regulators

[Michał Mirosław]

Workaround regulators whose supply name happens to be the same as its
own name. This fixes boards that used to work before the early supply
resolving was removed. The error message is left in place so that
offending drivers can be detected.

Fixes: aea6cb99703e ("regulator: resolve supply after creating regulator")
Cc: sta...@vger.kernel.org
Reported-by: Ahmad Fatoum 
Signed-off-by: Michał Mirosław 
---
 drivers/regulator/core.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index ab922ed273f3..38ba579efe2b 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1844,7 +1844,10 @@ static int regulator_resolve_supply(struct regulator_dev 
*rdev)
if (r == rdev) {
dev_err(dev, "Supply for %s (%s) resolved to itself\n",
rdev->desc->name, rdev->supply_name);
-   return -EINVAL;
+   if (!have_full_constraints())
+   return -EINVAL;
+   r = dummy_regulator_rdev;
+   get_device(>dev);
}
 
/*
-- 
2.20.1

[PATCH RESEND 2/4] regulator: debug early supply resolving

[Michał Mirosław]

Help debugging the case when set_machine_constraints() needs to be
repeated.

Signed-off-by: Michał Mirosław 
---
 drivers/regulator/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index bcd64ba21fb9..ad36f03d7ee6 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -5296,6 +5296,8 @@ regulator_register(const struct regulator_desc 
*regulator_desc,
/* FIXME: this currently triggers a chicken-and-egg problem
 * when creating -SUPPLY symlink in sysfs to a regulator
 * that is just being created */
+   rdev_dbg(rdev, "will resolve supply early: %s\n",
+rdev->supply_name);
ret = regulator_resolve_supply(rdev);
if (!ret)
ret = set_machine_constraints(rdev);
-- 
2.20.1

[PATCH RESEND 1/4] regulator: fix memory leak with repeated set_machine_constraints()

[Michał Mirosław]

Fixed commit introduced a possible second call to
set_machine_constraints() and that allocates memory for
rdev->constraints. Move the allocation to the caller so
it's easier to manage and done once.

Fixes: aea6cb99703e ("regulator: resolve supply after creating regulator")
Cc: sta...@vger.kernel.org
Signed-off-by: Michał Mirosław 
---
 drivers/regulator/core.c | 29 +
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 2e1ea18221ef..bcd64ba21fb9 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1315,7 +1315,6 @@ static int _regulator_do_enable(struct regulator_dev 
*rdev);
 /**
  * set_machine_constraints - sets regulator constraints
  * @rdev: regulator source
- * @constraints: constraints to apply
  *
  * Allows platform initialisation code to define and constrain
  * regulator circuits e.g. valid voltage/current ranges, etc.  NOTE:
@@ -1323,21 +1322,11 @@ static int _regulator_do_enable(struct regulator_dev 
*rdev);
  * regulator operations to proceed i.e. set_voltage, set_current_limit,
  * set_mode.
  */
-static int set_machine_constraints(struct regulator_dev *rdev,
-   const struct regulation_constraints *constraints)
+static int set_machine_constraints(struct regulator_dev *rdev)
 {
int ret = 0;
const struct regulator_ops *ops = rdev->desc->ops;
 
-   if (constraints)
-   rdev->constraints = kmemdup(constraints, sizeof(*constraints),
-   GFP_KERNEL);
-   else
-   rdev->constraints = kzalloc(sizeof(*constraints),
-   GFP_KERNEL);
-   if (!rdev->constraints)
-   return -ENOMEM;
-
ret = machine_constraints_voltage(rdev, rdev->constraints);
if (ret != 0)
return ret;
@@ -5146,7 +5135,6 @@ struct regulator_dev *
 regulator_register(const struct regulator_desc *regulator_desc,
   const struct regulator_config *cfg)
 {
-   const struct regulation_constraints *constraints = NULL;
const struct regulator_init_data *init_data;
struct regulator_config *config = NULL;
static atomic_t regulator_no = ATOMIC_INIT(-1);
@@ -5285,14 +5273,23 @@ regulator_register(const struct regulator_desc 
*regulator_desc,
 
/* set regulator constraints */
if (init_data)
-   constraints = _data->constraints;
+   rdev->constraints = kmemdup(_data->constraints,
+   sizeof(*rdev->constraints),
+   GFP_KERNEL);
+   else
+   rdev->constraints = kzalloc(sizeof(*rdev->constraints),
+   GFP_KERNEL);
+   if (!rdev->constraints) {
+   ret = -ENOMEM;
+   goto wash;
+   }
 
if (init_data && init_data->supply_regulator)
rdev->supply_name = init_data->supply_regulator;
else if (regulator_desc->supply_name)
rdev->supply_name = regulator_desc->supply_name;
 
-   ret = set_machine_constraints(rdev, constraints);
+   ret = set_machine_constraints(rdev);
if (ret == -EPROBE_DEFER) {
/* Regulator might be in bypass mode and so needs its supply
 * to set the constraints */
@@ -5301,7 +5298,7 @@ regulator_register(const struct regulator_desc 
*regulator_desc,
 * that is just being created */
ret = regulator_resolve_supply(rdev);
if (!ret)
-   ret = set_machine_constraints(rdev, constraints);
+   ret = set_machine_constraints(rdev);
else
rdev_dbg(rdev, "unable to resolve supply early: %pe\n",
 ERR_PTR(ret));
-- 
2.20.1

[PATCH 4/4] regulator: workaround self-referent regulators

[Michał Mirosław]

Workaround regulators whose supply name happens to be the same as its
own name. This fixes boards that used to work before the early supply
resolving was removed. The error message is left in place so that
offending drivers can be detected.

Fixes: aea6cb99703e ("regulator: resolve supply after creating regulator")
Cc: sta...@vger.kernel.org
Reported-by: Ahmad Fatoum 
Signed-off-by: Michał Mirosław 
---
 drivers/regulator/core.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index ab922ed273f3..38ba579efe2b 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1844,7 +1844,10 @@ static int regulator_resolve_supply(struct regulator_dev 
*rdev)
if (r == rdev) {
dev_err(dev, "Supply for %s (%s) resolved to itself\n",
rdev->desc->name, rdev->supply_name);
-   return -EINVAL;
+   if (!have_full_constraints())
+   return -EINVAL;
+   r = dummy_regulator_rdev;
+   get_device(>dev);
}
 
/*
-- 
2.20.1

[PATCH 3/4] regulator: avoid resolve_supply() infinite recursion

[Michał Mirosław]

When a regulator's name equals its supply's name the
regulator_resolve_supply() recurses indefinitely. Add a check
so that debugging the problem is easier. The "fixed" commit
just exposed the problem.

Fixes: aea6cb99703e ("regulator: resolve supply after creating regulator")
Cc: sta...@vger.kernel.org
Reported-by: Ahmad Fatoum 
Signed-off-by: Michał Mirosław 
---
 drivers/regulator/core.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index ad36f03d7ee6..ab922ed273f3 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1841,6 +1841,12 @@ static int regulator_resolve_supply(struct regulator_dev 
*rdev)
}
}
 
+   if (r == rdev) {
+   dev_err(dev, "Supply for %s (%s) resolved to itself\n",
+   rdev->desc->name, rdev->supply_name);
+   return -EINVAL;
+   }
+
/*
 * If the supply's parent device is not the same as the
 * regulator's parent device, then ensure the parent device
-- 
2.20.1

[PATCH 2/4] regulator: debug early supply resolving

[Michał Mirosław]

Help debugging the case when set_machine_constraints() needs to be
repeated.

Signed-off-by: Michał Mirosław 
---
 drivers/regulator/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index bcd64ba21fb9..ad36f03d7ee6 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -5296,6 +5296,8 @@ regulator_register(const struct regulator_desc 
*regulator_desc,
/* FIXME: this currently triggers a chicken-and-egg problem
 * when creating -SUPPLY symlink in sysfs to a regulator
 * that is just being created */
+   rdev_dbg(rdev, "will resolve supply early: %s\n",
+rdev->supply_name);
ret = regulator_resolve_supply(rdev);
if (!ret)
ret = set_machine_constraints(rdev);
-- 
2.20.1

[PATCH 0/4] regulator: debugging and fixing supply deps

[Michał Mirosław]

It turns out that commit aea6cb99703e ("regulator: resolve supply
after creating regulator") exposed a number of issues in regulator
initialization and introduced a memory leak of its own. One uncovered
problem was already fixed by cf1ad559a20d ("regulator: defer probe when
trying to get voltage from unresolved supply"). This series fixes the
remaining ones and adds a two debugging aids to help in the future.

The final patch adds a workaround to preexisting problem occurring with
regulators that have the same name as its supply_name. This worked
before by accident, so might be worth backporting. The error message is
left on purpose so that these configurations can be detected and fixed.

(The first two patches are resends from Nov 5).

Michał Mirosław (4):
  regulator: fix memory leak with repeated set_machine_constraints()
  regulator: debug early supply resolving
  regulator: avoid resolve_supply() infinite recursion
  regulator: workaround self-referent regulators

 drivers/regulator/core.c | 40 
 1 file changed, 24 insertions(+), 16 deletions(-)

-- 
2.20.1

[PATCH 1/4] regulator: fix memory leak with repeated set_machine_constraints()

[Michał Mirosław]

Fixed commit introduced a possible second call to
set_machine_constraints() and that allocates memory for
rdev->constraints. Move the allocation to the caller so
it's easier to manage and done once.

Fixes: aea6cb99703e ("regulator: resolve supply after creating regulator")
Cc: sta...@vger.kernel.org
Signed-off-by: Michał Mirosław 
---
 drivers/regulator/core.c | 29 +
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 2e1ea18221ef..bcd64ba21fb9 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1315,7 +1315,6 @@ static int _regulator_do_enable(struct regulator_dev 
*rdev);
 /**
  * set_machine_constraints - sets regulator constraints
  * @rdev: regulator source
- * @constraints: constraints to apply
  *
  * Allows platform initialisation code to define and constrain
  * regulator circuits e.g. valid voltage/current ranges, etc.  NOTE:
@@ -1323,21 +1322,11 @@ static int _regulator_do_enable(struct regulator_dev 
*rdev);
  * regulator operations to proceed i.e. set_voltage, set_current_limit,
  * set_mode.
  */
-static int set_machine_constraints(struct regulator_dev *rdev,
-   const struct regulation_constraints *constraints)
+static int set_machine_constraints(struct regulator_dev *rdev)
 {
int ret = 0;
const struct regulator_ops *ops = rdev->desc->ops;
 
-   if (constraints)
-   rdev->constraints = kmemdup(constraints, sizeof(*constraints),
-   GFP_KERNEL);
-   else
-   rdev->constraints = kzalloc(sizeof(*constraints),
-   GFP_KERNEL);
-   if (!rdev->constraints)
-   return -ENOMEM;
-
ret = machine_constraints_voltage(rdev, rdev->constraints);
if (ret != 0)
return ret;
@@ -5146,7 +5135,6 @@ struct regulator_dev *
 regulator_register(const struct regulator_desc *regulator_desc,
   const struct regulator_config *cfg)
 {
-   const struct regulation_constraints *constraints = NULL;
const struct regulator_init_data *init_data;
struct regulator_config *config = NULL;
static atomic_t regulator_no = ATOMIC_INIT(-1);
@@ -5285,14 +5273,23 @@ regulator_register(const struct regulator_desc 
*regulator_desc,
 
/* set regulator constraints */
if (init_data)
-   constraints = _data->constraints;
+   rdev->constraints = kmemdup(_data->constraints,
+   sizeof(*rdev->constraints),
+   GFP_KERNEL);
+   else
+   rdev->constraints = kzalloc(sizeof(*rdev->constraints),
+   GFP_KERNEL);
+   if (!rdev->constraints) {
+   ret = -ENOMEM;
+   goto wash;
+   }
 
if (init_data && init_data->supply_regulator)
rdev->supply_name = init_data->supply_regulator;
else if (regulator_desc->supply_name)
rdev->supply_name = regulator_desc->supply_name;
 
-   ret = set_machine_constraints(rdev, constraints);
+   ret = set_machine_constraints(rdev);
if (ret == -EPROBE_DEFER) {
/* Regulator might be in bypass mode and so needs its supply
 * to set the constraints */
@@ -5301,7 +5298,7 @@ regulator_register(const struct regulator_desc 
*regulator_desc,
 * that is just being created */
ret = regulator_resolve_supply(rdev);
if (!ret)
-   ret = set_machine_constraints(rdev, constraints);
+   ret = set_machine_constraints(rdev);
else
rdev_dbg(rdev, "unable to resolve supply early: %pe\n",
 ERR_PTR(ret));
-- 
2.20.1

[PATCH RESEND v8 1/4] input: elants: document some registers and values

[Michał Mirosław]

Add information found in downstream kernels, to make the code less
magic.

Signed-off-by: Michał Mirosław 
Reviewed-by: Dmitry Osipenko 
Tested-by: Dmitry Osipenko 
---
 drivers/input/touchscreen/elants_i2c.c | 18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/elants_i2c.c 
b/drivers/input/touchscreen/elants_i2c.c
index 50c348297e38..d51cb910fba1 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -82,7 +82,7 @@
 
 #define HEADER_REPORT_10_FINGER0x62
 
-/* Header (4 bytes) plus 3 fill 10-finger packets */
+/* Header (4 bytes) plus 3 full 10-finger packets */
 #define MAX_PACKET_SIZE169
 
 #define BOOT_TIME_DELAY_MS 50
@@ -97,6 +97,10 @@
 #define E_INFO_PHY_SCAN0xD7
 #define E_INFO_PHY_DRIVER  0xD8
 
+/* FW write command, 0x54 0x?? 0x0, 0x01 */
+#define E_POWER_STATE_SLEEP0x50
+#define E_POWER_STATE_RESUME   0x58
+
 #define MAX_RETRIES3
 #define MAX_FW_UPDATE_RETRIES  30
 
@@ -269,8 +273,8 @@ static int elants_i2c_calibrate(struct elants_data *ts)
 {
struct i2c_client *client = ts->client;
int ret, error;
-   static const u8 w_flashkey[] = { 0x54, 0xC0, 0xE1, 0x5A };
-   static const u8 rek[] = { 0x54, 0x29, 0x00, 0x01 };
+   static const u8 w_flashkey[] = { CMD_HEADER_WRITE, 0xC0, 0xE1, 0x5A };
+   static const u8 rek[] = { CMD_HEADER_WRITE, 0x29, 0x00, 0x01 };
static const u8 rek_resp[] = { CMD_HEADER_REK, 0x66, 0x66, 0x66 };
 
disable_irq(client->irq);
@@ -1388,7 +1392,9 @@ static int __maybe_unused elants_i2c_suspend(struct 
device *dev)
 {
struct i2c_client *client = to_i2c_client(dev);
struct elants_data *ts = i2c_get_clientdata(client);
-   const u8 set_sleep_cmd[] = { 0x54, 0x50, 0x00, 0x01 };
+   const u8 set_sleep_cmd[] = {
+   CMD_HEADER_WRITE, E_POWER_STATE_SLEEP, 0x00, 0x01
+   };
int retry_cnt;
int error;
 
@@ -1425,7 +1431,9 @@ static int __maybe_unused elants_i2c_resume(struct device 
*dev)
 {
struct i2c_client *client = to_i2c_client(dev);
struct elants_data *ts = i2c_get_clientdata(client);
-   const u8 set_active_cmd[] = { 0x54, 0x58, 0x00, 0x01 };
+   const u8 set_active_cmd[] = {
+   CMD_HEADER_WRITE, E_POWER_STATE_RESUME, 0x00, 0x01
+   };
int retry_cnt;
int error;
 
-- 
2.20.1

[PATCH RESEND v8 3/4] input: elants: read touchscreen size for EKTF3624

[Michał Mirosław]

EKTF3624 as present in Asus TF300T tablet has touchscreen size encoded
in different registers.

Signed-off-by: Michał Mirosław 
Reviewed-by: Dmitry Osipenko 
Tested-by: Dmitry Osipenko 
---
 drivers/input/touchscreen/elants_i2c.c | 84 --
 1 file changed, 79 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/elants_i2c.c 
b/drivers/input/touchscreen/elants_i2c.c
index f1bf3e000e96..c24d8cdc4251 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -35,7 +35,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
@@ -43,6 +43,10 @@
 /* Device, Driver information */
 #define DEVICE_NAME"elants_i2c"
 
+/* Device IDs */
+#define EKTH3500   0
+#define EKTF3624   1
+
 /* Convert from rows or columns into resolution */
 #define ELAN_TS_RESOLUTION(n, m)   (((n) - 1) * (m))
 
@@ -94,6 +98,8 @@
 #define E_ELAN_INFO_REK0xD0
 #define E_ELAN_INFO_TEST_VER   0xE0
 #define E_ELAN_INFO_FW_ID  0xF0
+#define E_INFO_X_RES   0x60
+#define E_INFO_Y_RES   0x63
 #define E_INFO_OSR 0xD6
 #define E_INFO_PHY_SCAN0xD7
 #define E_INFO_PHY_DRIVER  0xD8
@@ -157,6 +163,7 @@ struct elants_data {
 
bool wake_irq_enabled;
bool keep_power_in_suspend;
+   u8 chip_id;
 
/* Must be last to be used for DMA operations */
u8 buf[MAX_PACKET_SIZE] cacheline_aligned;
@@ -434,7 +441,58 @@ static int elants_i2c_query_bc_version(struct elants_data 
*ts)
return 0;
 }
 
-static int elants_i2c_query_ts_info(struct elants_data *ts)
+static int elants_i2c_query_ts_info_ektf(struct elants_data *ts)
+{
+   struct i2c_client *client = ts->client;
+   int error;
+   u8 resp[4];
+   u16 phy_x, phy_y;
+   const u8 get_xres_cmd[] = {
+   CMD_HEADER_READ, E_INFO_X_RES, 0x00, 0x00
+   };
+   const u8 get_yres_cmd[] = {
+   CMD_HEADER_READ, E_INFO_Y_RES, 0x00, 0x00
+   };
+
+   /* Get X/Y size in mm */
+   error = elants_i2c_execute_command(client, get_xres_cmd,
+  sizeof(get_xres_cmd),
+  resp, sizeof(resp), 1,
+  "get X size");
+   if (error)
+   return error;
+
+   phy_x = resp[2] | ((resp[3] & 0xF0) << 4);
+
+   error = elants_i2c_execute_command(client, get_yres_cmd,
+  sizeof(get_yres_cmd),
+  resp, sizeof(resp), 1,
+  "get Y size");
+   if (error)
+   return error;
+
+   phy_y = resp[2] | ((resp[3] & 0xF0) << 4);
+
+   if (!phy_x || !phy_y) {
+   dev_warn(>dev,
+"invalid size data: %d x %d mm\n",
+phy_x, phy_y);
+   return 0;
+   }
+
+   dev_dbg(>dev, "phy_x=%d, phy_y=%d\n", phy_x, phy_y);
+
+   /* calculate resolution from size */
+   ts->x_max = 2240-1;
+   ts->x_res = DIV_ROUND_CLOSEST(ts->prop.max_x, phy_x);
+
+   ts->y_max = 1408-1;
+   ts->y_res = DIV_ROUND_CLOSEST(ts->prop.max_y, phy_y);
+
+   return 0;
+}
+
+static int elants_i2c_query_ts_info_ekth(struct elants_data *ts)
 {
struct i2c_client *client = ts->client;
int error;
@@ -588,8 +646,20 @@ static int elants_i2c_initialize(struct elants_data *ts)
error = elants_i2c_query_fw_version(ts);
if (!error)
error = elants_i2c_query_test_version(ts);
-   if (!error)
-   error = elants_i2c_query_ts_info(ts);
+
+   switch (ts->chip_id) {
+   case EKTH3500:
+   if (!error)
+   error = elants_i2c_query_ts_info_ekth(ts);
+   break;
+   case EKTF3624:
+   if (!error)
+   error = elants_i2c_query_ts_info_ektf(ts);
+   break;
+   default:
+   unreachable();
+   break;
+   }
 
if (error)
ts->iap_mode = ELAN_IAP_RECOVERY;
@@ -1266,6 +1336,9 @@ static int elants_i2c_probe(struct i2c_client *client,
ts->client = client;
i2c_set_clientdata(client, ts);
 
+   if (client->dev.of_node)
+   ts->chip_id = (uintptr_t)of_device_get_match_data(>dev);
+
ts->vcc33 = devm_regulator_get(>dev, "vcc33");
if (IS_ERR(ts->vcc33)) {
error = PTR_ERR(ts->vcc33);
@@ -1495,7 +1568,8 @@ MODULE_DEVICE_TABLE(acpi, elants_acpi_id);
 
 #ifdef CONFIG_OF
 static const struct of_device_id elants_of_match[] = {
-   { .compatible = "elan,ekth3500" },
+   { .compatible = "elan,ekth3500", .data = 

[PATCH RESEND v8 0/4] input: elants: Support Asus TF300T and Nexus 7 touchscreens

[Michał Mirosław]

This series cleans up the driver a bit and implements changes needed to
support EKTF3624-based touchscreen used in Asus TF300T, Google Nexus 7
and similar Tegra3-based tablets.

---
v2: extended with Dmitry's patches (replaced v1 patches 3 and 4)
v3: rebased for v5.7-rc1
v4: rebased onto v5.7-rc2+ (current Linus' master)
update "remove unused axes" and "refactor
  elants_i2c_execute_command()" patches after review
add David's patch converting DT binding to YAML
v5: rebased onto dtor/input/for-linus
v6: rebased onto newer dtor/input/for-linus
remove yet unused constants from patch 1
added a new drive-by cleanup (last patch)
v7: rebased onto current dtor/input/for-next
v8: rebased onto current dtor/input/for-linus
---

Dmitry Osipenko (1):
  input: elants: support 0x66 reply opcode for reporting touches

Michał Mirosław (3):
  input: elants: document some registers and values
  input: elants: support old touch report format
  input: elants: read touchscreen size for EKTF3624

 drivers/input/touchscreen/elants_i2c.c | 149 +
 1 file changed, 127 insertions(+), 22 deletions(-)

-- 
2.20.1

[PATCH RESEND v8 4/4] input: elants: support 0x66 reply opcode for reporting touches

[Michał Mirosław]

From: Dmitry Osipenko 

eKTF3624 touchscreen firmware uses two variants of the reply opcodes for
reporting touch events: one is 0x63 (used by older firmware) and other is
0x66 (used by newer firmware). The 0x66 variant is equal to 0x63 of
eKTH3500, while 0x63 needs small adjustment of the touch pressure value.

Nexus 7 tablet device has eKTF3624 touchscreen and it uses 0x66 opcode for
reporting touch events, let's support it now. Other devices, eg. ASUS TF300T,
use 0x63.

Note: CMD_HEADER_REK is used for replying to calibration requests, it has
the same 0x66 opcode number which eKTF3624 uses for reporting touches.
The calibration replies are handled separately from the the rest of the
commands in the driver by entering into ELAN_WAIT_RECALIBRATION state
and thus this change shouldn't change the old behavior.

Signed-off-by: Dmitry Osipenko 
Tested-by: Michał Mirosław 
Signed-off-by: Michał Mirosław 
---
 drivers/input/touchscreen/elants_i2c.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/elants_i2c.c 
b/drivers/input/touchscreen/elants_i2c.c
index c24d8cdc4251..1cbda6f20d07 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -61,6 +61,15 @@
 #define QUEUE_HEADER_NORMAL0X63
 #define QUEUE_HEADER_WAIT  0x64
 
+/*
+ * Depending on firmware version, eKTF3624 touchscreens may utilize one of
+ * these opcodes for the touch events: 0x63 and 0x66. The 0x63 is used by
+ * older firmware version and differs from 0x66 such that touch pressure
+ * value needs to be adjusted. The 0x66 opcode of newer firmware is equal
+ * to 0x63 of eKTH3500.
+ */
+#define QUEUE_HEADER_NORMAL2   0x66
+
 /* Command header definition */
 #define CMD_HEADER_WRITE   0x54
 #define CMD_HEADER_READ0x53
@@ -1052,7 +1061,6 @@ static irqreturn_t elants_i2c_irq(int irq, void *_dev)
switch (ts->buf[FW_HDR_TYPE]) {
case CMD_HEADER_HELLO:
case CMD_HEADER_RESP:
-   case CMD_HEADER_REK:
break;
 
case QUEUE_HEADER_WAIT:
@@ -1072,6 +1080,7 @@ static irqreturn_t elants_i2c_irq(int irq, void *_dev)
break;
 
case QUEUE_HEADER_NORMAL:
+   case QUEUE_HEADER_NORMAL2:
report_count = ts->buf[FW_HDR_COUNT];
if (report_count == 0 || report_count > 3) {
dev_err(>dev,
-- 
2.20.1

[PATCH RESEND v8 2/4] input: elants: support old touch report format

[Michał Mirosław]

Support ELAN touchpad sensor with older firmware as found on eg. Asus
Transformer Pads.

Signed-off-by: Michał Mirosław 
Reviewed-by: Dmitry Osipenko 
Tested-by: Dmitry Osipenko 
---
 drivers/input/touchscreen/elants_i2c.c | 36 ++
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/input/touchscreen/elants_i2c.c 
b/drivers/input/touchscreen/elants_i2c.c
index d51cb910fba1..f1bf3e000e96 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -69,6 +69,7 @@
 #define CMD_HEADER_REK 0x66
 
 /* FW position data */
+#define PACKET_SIZE_OLD40
 #define PACKET_SIZE55
 #define MAX_CONTACT_NUM10
 #define FW_POS_HEADER  0
@@ -853,7 +854,8 @@ static int elants_i2c_fw_update(struct elants_data *ts)
  * Event reporting.
  */
 
-static void elants_i2c_mt_event(struct elants_data *ts, u8 *buf)
+static void elants_i2c_mt_event(struct elants_data *ts, u8 *buf,
+   size_t report_len)
 {
struct input_dev *input = ts->input;
unsigned int n_fingers;
@@ -866,7 +868,8 @@ static void elants_i2c_mt_event(struct elants_data *ts, u8 
*buf)
buf[FW_POS_STATE];
 
dev_dbg(>client->dev,
-   "n_fingers: %u, state: %04x\n",  n_fingers, finger_state);
+   "n_fingers: %u, state: %04x, report_len: %zu\n",
+   n_fingers, finger_state, report_len);
 
/* Note: all fingers have the same tool type */
tool_type = buf[FW_POS_TOOL_TYPE] & BIT(0) ?
@@ -880,8 +883,16 @@ static void elants_i2c_mt_event(struct elants_data *ts, u8 
*buf)
pos = [FW_POS_XY + i * 3];
x = (((u16)pos[0] & 0xf0) << 4) | pos[1];
y = (((u16)pos[0] & 0x0f) << 8) | pos[2];
-   p = buf[FW_POS_PRESSURE + i];
-   w = buf[FW_POS_WIDTH + i];
+   if (report_len == PACKET_SIZE_OLD) {
+   w = buf[FW_POS_WIDTH + i / 2];
+   w >>= 4 * (~i & 1); // little-endian-nibbles
+   w |= w << 4;
+   w |= !w;
+   p = w;
+   } else {
+   p = buf[FW_POS_PRESSURE + i];
+   w = buf[FW_POS_WIDTH + i];
+   }
 
dev_dbg(>client->dev, "i=%d x=%d y=%d p=%d w=%d\n",
i, x, y, p, w);
@@ -913,7 +924,8 @@ static u8 elants_i2c_calculate_checksum(u8 *buf)
return checksum;
 }
 
-static void elants_i2c_event(struct elants_data *ts, u8 *buf)
+static void elants_i2c_event(struct elants_data *ts, u8 *buf,
+size_t report_len)
 {
u8 checksum = elants_i2c_calculate_checksum(buf);
 
@@ -927,7 +939,7 @@ static void elants_i2c_event(struct elants_data *ts, u8 
*buf)
 "%s: unknown packet type: %02x\n",
 __func__, buf[FW_POS_HEADER]);
else
-   elants_i2c_mt_event(ts, buf);
+   elants_i2c_mt_event(ts, buf, report_len);
 }
 
 static irqreturn_t elants_i2c_irq(int irq, void *_dev)
@@ -985,7 +997,8 @@ static irqreturn_t elants_i2c_irq(int irq, void *_dev)
break;
 
case QUEUE_HEADER_SINGLE:
-   elants_i2c_event(ts, >buf[HEADER_SIZE]);
+   elants_i2c_event(ts, >buf[HEADER_SIZE],
+ts->buf[FW_HDR_LENGTH]);
break;
 
case QUEUE_HEADER_NORMAL:
@@ -998,17 +1011,18 @@ static irqreturn_t elants_i2c_irq(int irq, void *_dev)
}
 
report_len = ts->buf[FW_HDR_LENGTH] / report_count;
-   if (report_len != PACKET_SIZE) {
+   if (report_len != PACKET_SIZE &&
+   report_len != PACKET_SIZE_OLD) {
dev_err(>dev,
-   "mismatching report length: %*ph\n",
+   "unsupported report length: %*ph\n",
HEADER_SIZE, ts->buf);
break;
}
 
for (i = 0; i < report_count; i++) {
u8 *buf = ts->buf + HEADER_SIZE +
-   i * PACKET_SIZE;
-   elants_i2c_event(ts, buf);
+ i * report_len;
+   elants_i2c_event(ts, buf, report_len);
}
break;
 
-- 
2.20.1

Re: About regression caused by commit aea6cb99703e ("regulator: resolve supply after creating regulator")

[Michał Mirosław]

On Sun, Nov 08, 2020 at 03:35:33PM +0800, Qu Wenruo wrote:
> Hi Michał,
> 
> Recently when testing v5.10-rc2, I found my RK3399 boards failed to boot
> from NVME.
> 
> It turns out that, commit aea6cb99703e ("regulator: resolve supply after
> creating regulator") seems to be the cause.
> 
> In RK3399 board, vpcie1v8 and vpcie0v9 of the pcie controller is
> provided by RK808 regulator.
> With that commit, now RK808 regulator fails to register:
> 
> [1.402500] rk808-regulator rk808-regulator: there is no dvs0 gpio
> [1.403104] rk808-regulator rk808-regulator: there is no dvs1 gpio
> [1.419856] rk808 0-001b: failed to register 12 regulator
> [1.422801] rk808-regulator: probe of rk808-regulator failed with
> error -22

Hi,

This looks lika the problem fixed by commit cf1ad559a20d ("regulator: defer
probe when trying to get voltage from unresolved supply") recently accepted
to regulator tree [1]. Can you verify this?

[1] git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next 
 
Best Regards
Michał Mirosław

Re: [BUG] Error applying setting, reverse things back on lot of devices

[Michał Mirosław]

On Thu, Nov 05, 2020 at 10:11:30AM +0100, Ahmad Fatoum wrote:
> Hello,
> 
> On 11/5/20 3:57 AM, Michał Mirosław wrote:
> >>> Can you catch debug logs for the bootup in question? I'm not sure what's
> >>> the failure mode in your case. I guess this is not a bypassed regulator?
> >>
> >> Boot up with v5.10-rc2 + your cf1ad559a2 ("regulator: defer probe when 
> >> trying
> >> to get voltage from unresolved supply") hangs:
> >>
> >> [1.151489] stm32f7-i2c 40015000.i2c: STM32F7 I2C-0 bus adapter
> >> [1.180698] stpmic1 1-0033: PMIC Chip Version: 0x10
> >> [1.189526] vddcore: supplied by regulator-dummy
> >> [1.195633] vdd_ddr: supplied by regulator-dummy
> >> [1.201672] vdd: supplied by regulator-dummy
> >> [1.207452] v3v3: supplied by 5V2
> >> [1.211997] v1v8_audio: supplied by v3v3
> >> [1.218036] v3v3_hdmi: supplied by 5V2
> >> [1.223626] vtt_ddr: supplied by regulator-dummy
> >> [1.227107] vdd_usb: supplied by regulator-dummy
> >> [1.234532] vdda: supplied by 5V2
> >> [1.239497] v1v2_hdmi: supplied by v3v3
> > [...]
> > 
> > Can you try with the patches I just sent and with debug logs enabled?
> > 
> > The first one just plugs a memory leak, but if there is some state
> > changed/saved in the rdev->constraints (can't find that code, though),
> > this might prevent it from being overwritten.
> > 
> > The second patch will just tell us if you hit the early resolve case.
> 
> Problem still persists. Early resolve case not hit:
[...]
> [1.594492] vref_ddr: at 500 mV, enabled
> [1.597047] edt_ft5x06 0-0038: touchscreen probe failed
> [1.597211] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: Looking 
> up vref_ddr-supply from device tree
> [1.612406] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: Looking 
> up vref_ddr-supply property in node /soc/i2c@5c002000/stpmic@33/regulators 
> failed
> 
>   [ snip - continues many times ]
> 
> [6.699244] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: Looking 
> up vref_ddr-supply property in node /soc/i2c@5c002000/stpmic@33/regulators 
> failed
> [    6.713312] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: Looking 
> up vref_ddr-supply from device tree

It seems that final regulator_resolve_supply() is spinning recursively.
Is the regulator name the same as its supply_name? Can you try the patch
below to verify this?

Best Regards
Michał Mirosław

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index c84e3b0b63de..983a4bd3e98c 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1798,6 +1798,8 @@ static int regulator_resolve_supply(struct regulator_dev 
*rdev)
if (rdev->supply)
return 0;
 
+   dev_dbg(dev, "Resolving supply %s for %s\n", rdev->supply_name, 
rdev->desc->name);
+
r = regulator_dev_lookup(dev, rdev->supply_name);
if (IS_ERR(r)) {
ret = PTR_ERR(r);
@@ -1816,6 +1818,12 @@ static int regulator_resolve_supply(struct regulator_dev 
*rdev)
}
}
 
+   if (r == rdev) {
+   dev_err(dev, "Supply for %s (%s) resolved to itself\n",
+   rdev->desc->name, rdev->supply_name);
+   return -EINVAL;
+   }
+
/*
 * If the supply's parent device is not the same as the
 * regulator's parent device, then ensure the parent device

Re: [BUG] Error applying setting, reverse things back on lot of devices

[Michał Mirosław]

On Wed, Nov 04, 2020 at 11:28:45AM +0100, Ahmad Fatoum wrote:
> Hello,
> 
> On 11/2/20 9:27 PM, Michał Mirosław wrote:
> > On Mon, Nov 02, 2020 at 01:48:54PM +0100, Ahmad Fatoum wrote:
> >> Hello Michał,
> >>
> >> CC += linux-stm32
> >>
> >> On 10/24/20 1:53 PM, Michał Mirosław wrote:
> >>> On Fri, Oct 23, 2020 at 10:39:43PM +0200, Corentin Labbe wrote:
> >>>> On Fri, Oct 23, 2020 at 03:42:01PM +0200, Corentin Labbe wrote:
> >>>>> On Wed, Oct 21, 2020 at 08:31:49PM +0200, Corentin Labbe wrote:
> >>>>> I have just saw thoses 3 lines which are probably the real problem.
> >>>>> I have started a new bisect with this error, but it is hitting the same 
> >>>>> "crash range" the first one.
> >>>>>
> >>>>
> >>>> I have bisected the problem to commit 
> >>>> aea6cb99703e17019e025aa71643b4d3e0a24413 ("regulator: resolve supply 
> >>>> after creating regulator")
> >>>> Reverting this fix my problem.
> >>
> >> The change broke boot on all the STM32MP1 boards, because the STPMIC driver
> >> has a vref_ddr regulator, which does not have a dedicated supply, but 
> >> without
> >> a vref_ddr-supply property the system now no longer boots.
> > [...]
> > 
> > Can you catch debug logs for the bootup in question? I'm not sure what's
> > the failure mode in your case. I guess this is not a bypassed regulator?
> 
> Boot up with v5.10-rc2 + your cf1ad559a2 ("regulator: defer probe when trying
> to get voltage from unresolved supply") hangs:
> 
> [1.151489] stm32f7-i2c 40015000.i2c: STM32F7 I2C-0 bus adapter
> [1.180698] stpmic1 1-0033: PMIC Chip Version: 0x10
> [1.189526] vddcore: supplied by regulator-dummy
> [1.195633] vdd_ddr: supplied by regulator-dummy
> [1.201672] vdd: supplied by regulator-dummy
> [1.207452] v3v3: supplied by 5V2
> [1.211997] v1v8_audio: supplied by v3v3
> [1.218036] v3v3_hdmi: supplied by 5V2
> [1.223626] vtt_ddr: supplied by regulator-dummy
> [1.227107] vdd_usb: supplied by regulator-dummy
> [1.234532] vdda: supplied by 5V2
> [1.239497] v1v2_hdmi: supplied by v3v3
[...]

Can you try with the patches I just sent and with debug logs enabled?

The first one just plugs a memory leak, but if there is some state
changed/saved in the rdev->constraints (can't find that code, though),
this might prevent it from being overwritten.

The second patch will just tell us if you hit the early resolve case.

Best Regards,
Michał Mirosław

[PATCH] regulator: debug early supply resolving

[Michał Mirosław]

Help debugging the case when set_machine_constraints() needs to be
repeated.

Signed-off-by: Michał Mirosław 
---
 drivers/regulator/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 402a72a70eb1..c84e3b0b63de 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -5271,6 +5271,8 @@ regulator_register(const struct regulator_desc 
*regulator_desc,
/* FIXME: this currently triggers a chicken-and-egg problem
 * when creating -SUPPLY symlink in sysfs to a regulator
 * that is just being created */
+   rdev_dbg(rdev, "will resolve supply early: %s\n",
+rdev->supply_name);
ret = regulator_resolve_supply(rdev);
if (!ret)
ret = set_machine_constraints(rdev);
-- 
2.20.1

[PATCH] regulator: fix memory leak with repeated set_machine_constraints()

[Michał Mirosław]

Fixed commit introduced a possible second call to
set_machine_constraints() and that allocates memory for
rdev->constraints. Move the allocation to the caller so
it's easier to manage and done once.

Fixes: aea6cb99703e ("regulator: resolve supply after creating regulator")
Cc: sta...@vger.kernel.org
Signed-off-by: Michał Mirosław 
---
 drivers/regulator/core.c | 29 +
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 8521f74ee75c..402a72a70eb1 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1290,7 +1290,6 @@ static int _regulator_do_enable(struct regulator_dev 
*rdev);
 /**
  * set_machine_constraints - sets regulator constraints
  * @rdev: regulator source
- * @constraints: constraints to apply
  *
  * Allows platform initialisation code to define and constrain
  * regulator circuits e.g. valid voltage/current ranges, etc.  NOTE:
@@ -1298,21 +1297,11 @@ static int _regulator_do_enable(struct regulator_dev 
*rdev);
  * regulator operations to proceed i.e. set_voltage, set_current_limit,
  * set_mode.
  */
-static int set_machine_constraints(struct regulator_dev *rdev,
-   const struct regulation_constraints *constraints)
+static int set_machine_constraints(struct regulator_dev *rdev)
 {
int ret = 0;
const struct regulator_ops *ops = rdev->desc->ops;
 
-   if (constraints)
-   rdev->constraints = kmemdup(constraints, sizeof(*constraints),
-   GFP_KERNEL);
-   else
-   rdev->constraints = kzalloc(sizeof(*constraints),
-   GFP_KERNEL);
-   if (!rdev->constraints)
-   return -ENOMEM;
-
ret = machine_constraints_voltage(rdev, rdev->constraints);
if (ret != 0)
return ret;
@@ -5121,7 +5110,6 @@ struct regulator_dev *
 regulator_register(const struct regulator_desc *regulator_desc,
   const struct regulator_config *cfg)
 {
-   const struct regulation_constraints *constraints = NULL;
const struct regulator_init_data *init_data;
struct regulator_config *config = NULL;
static atomic_t regulator_no = ATOMIC_INIT(-1);
@@ -5260,14 +5248,23 @@ regulator_register(const struct regulator_desc 
*regulator_desc,
 
/* set regulator constraints */
if (init_data)
-   constraints = _data->constraints;
+   rdev->constraints = kmemdup(_data->constraints,
+   sizeof(*rdev->constraints),
+   GFP_KERNEL);
+   else
+   rdev->constraints = kzalloc(sizeof(*rdev->constraints),
+   GFP_KERNEL);
+   if (!rdev->constraints) {
+   ret = -ENOMEM;
+   goto wash;
+   }
 
if (init_data && init_data->supply_regulator)
rdev->supply_name = init_data->supply_regulator;
else if (regulator_desc->supply_name)
rdev->supply_name = regulator_desc->supply_name;
 
-   ret = set_machine_constraints(rdev, constraints);
+   ret = set_machine_constraints(rdev);
if (ret == -EPROBE_DEFER) {
/* Regulator might be in bypass mode and so needs its supply
 * to set the constraints */
@@ -5276,7 +5273,7 @@ regulator_register(const struct regulator_desc 
*regulator_desc,
 * that is just being created */
ret = regulator_resolve_supply(rdev);
if (!ret)
-   ret = set_machine_constraints(rdev, constraints);
+   ret = set_machine_constraints(rdev);
else
rdev_dbg(rdev, "unable to resolve supply early: %pe\n",
 ERR_PTR(ret));
-- 
2.20.1

Re: linux-next: build failure after merge of the mfd tree

[Michał Mirosław]

On Thu, Nov 05, 2020 at 12:50:27PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the mfd tree, today's linux-next build (arm
> multi_v7_defconfig) failed like this:
> 
> drivers/gpio/gpio-tps65910.c: In function 'tps65910_gpio_get':
> drivers/gpio/gpio-tps65910.c:31:2: error: implicit declaration of function 
> 'tps65910_reg_read' [-Werror=implicit-function-declaration]
>31 |  tps65910_reg_read(tps65910, TPS65910_GPIO0 + offset, );
>   |  ^
> drivers/gpio/gpio-tps65910.c: In function 'tps65910_gpio_set':
> drivers/gpio/gpio-tps65910.c:46:3: error: implicit declaration of function 
> 'tps65910_reg_set_bits' [-Werror=implicit-function-declaration]
>46 |   tps65910_reg_set_bits(tps65910, TPS65910_GPIO0 + offset,
>   |   ^
> drivers/gpio/gpio-tps65910.c:49:3: error: implicit declaration of function 
> 'tps65910_reg_clear_bits' [-Werror=implicit-function-declaration]
>49 |   tps65910_reg_clear_bits(tps65910, TPS65910_GPIO0 + offset,
>   |   ^~~
> 
> Caused by commit
> 
>   23feb2c3367c ("mfd: tps65910: Clean up after switching to regmap")
> 
> I have used the version of the mfd tree from next-20201104 for today.

Hi,

It's missing a patch for gpio part [1].

[1] https://lkml.org/lkml/2020/9/26/398

Best Regards
Michał Mirosław

Re: [PATCH 1/5] gpio: tps65910: use regmap accessors

[Michał Mirosław]

On Wed, Nov 04, 2020 at 02:43:31PM +, Lee Jones wrote:
> On Thu, 01 Oct 2020, Lee Jones wrote:
> > On Wed, 30 Sep 2020, Linus Walleij wrote:
> > > On Sun, Sep 27, 2020 at 1:59 AM Michał Mirosław  
> > > wrote:
> > > > Use regmap accessors directly for register manipulation - removing one
> > > > layer of abstraction.
> > > >
> > > > Signed-off-by: Michał Mirosław 
> > > Reviewed-by: Linus Walleij 
> > > 
> > > I suppose it is easiest that Lee apply all patches to the MFD tree?
> > Yes, that's fine.
> I think this patch is orthogonal right?
> 
> Not sure why it need to go in via MFD.
[...]

The patch 4 assumes all previous patches are applied (or there will be
build breakage).

Best Regards
Michał Mirosław

Re: [PATCH v1 00/30] Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs

[Michał Mirosław]

On Thu, Nov 05, 2020 at 02:43:57AM +0300, Dmitry Osipenko wrote:
> Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs, which reduces
> power consumption and heating of the Tegra chips. Tegra SoC has multiple
> hardware units which belong to a core power domain of the SoC and share
> the core voltage. The voltage must be selected in accordance to a minimum
> requirement of every core hardware unit.
[...]

Just looked briefly through the series - it looks like there is a lot of
code duplication in *_init_opp_table() functions. Could this be made
more generic / data-driven?

Best Regards
Michał Mirosław

Re: [PATCH v3] mfd: tps65910: Correct power-off programming sequence

[Michał Mirosław]

On Wed, Nov 04, 2020 at 04:44:08PM +0300, Dmitry Osipenko wrote:
> This patch fixes system shutdown on a devices that use TPS65910 as a
> system's power controller. In accordance to the TPS65910 datasheet, the
> PMIC's state-machine transitions into the OFF state only when DEV_OFF
> bit of DEVCTRL_REG is set. The ON / SLEEP states also should be cleared,
> otherwise PMIC won't get into a proper state on shutdown. Devices like
> Nexus 7 tablet and Ouya game console are now shutting down properly.
[...]

You might want to rebase on https://lkml.org/lkml/2020/9/26/397 as it's
probably going to be accepted shortly. This just means replacing
register accesses: tps65910_reg_*() -> regmap_*().

Best Regards,
Michał Mirosław

Re: [BUG] Error applying setting, reverse things back on lot of devices

[Michał Mirosław]

On Mon, Nov 02, 2020 at 01:48:54PM +0100, Ahmad Fatoum wrote:
> Hello Michał,
> 
> CC += linux-stm32
> 
> On 10/24/20 1:53 PM, Michał Mirosław wrote:
> > On Fri, Oct 23, 2020 at 10:39:43PM +0200, Corentin Labbe wrote:
> >> On Fri, Oct 23, 2020 at 03:42:01PM +0200, Corentin Labbe wrote:
> >>> On Wed, Oct 21, 2020 at 08:31:49PM +0200, Corentin Labbe wrote:
> >>> I have just saw thoses 3 lines which are probably the real problem.
> >>> I have started a new bisect with this error, but it is hitting the same 
> >>> "crash range" the first one.
> >>>
> >>
> >> I have bisected the problem to commit 
> >> aea6cb99703e17019e025aa71643b4d3e0a24413 ("regulator: resolve supply after 
> >> creating regulator")
> >> Reverting this fix my problem.
> 
> The change broke boot on all the STM32MP1 boards, because the STPMIC driver
> has a vref_ddr regulator, which does not have a dedicated supply, but without
> a vref_ddr-supply property the system now no longer boots.
[...]

Can you catch debug logs for the bootup in question? I'm not sure what's
the failure mode in your case. I guess this is not a bypassed regulator?

Best Regards,
Michał Mirosław

[PATCH v3 2/2] arm: defconfig: enable tegra20-spdif

[Michał Mirosław]

Enable tegra20-spdif in tegra and multiplatform defconfigs. The driver
will be switched to "default n" in another patch.

Signed-off-by: Michał Mirosław 
---
  v3: split from main patch
  v2: add the symbol to defconfig as suggested by Thierry Reding
---
 arch/arm/configs/multi_v7_defconfig | 1 +
 arch/arm/configs/tegra_defconfig| 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/arm/configs/multi_v7_defconfig 
b/arch/arm/configs/multi_v7_defconfig
index e731cdf7c88c..681742f81c08 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -745,6 +745,7 @@ CONFIG_SND_SOC_STM32_I2S=m
 CONFIG_SND_SUN4I_CODEC=m
 CONFIG_SND_SOC_TEGRA=m
 CONFIG_SND_SOC_TEGRA20_I2S=m
+CONFIG_SND_SOC_TEGRA20_SPDIF=m
 CONFIG_SND_SOC_TEGRA30_I2S=m
 CONFIG_SND_SOC_TEGRA_RT5640=m
 CONFIG_SND_SOC_TEGRA_WM8753=m
diff --git a/arch/arm/configs/tegra_defconfig b/arch/arm/configs/tegra_defconfig
index fff5fae0db30..08526eb50484 100644
--- a/arch/arm/configs/tegra_defconfig
+++ b/arch/arm/configs/tegra_defconfig
@@ -225,6 +225,7 @@ CONFIG_SND_HDA_CODEC_HDMI=y
 CONFIG_SND_SOC=y
 CONFIG_SND_SOC_TEGRA=y
 CONFIG_SND_SOC_TEGRA20_I2S=y
+CONFIG_SND_SOC_TEGRA20_SPDIF=y
 CONFIG_SND_SOC_TEGRA30_I2S=y
 CONFIG_SND_SOC_TEGRA_RT5640=y
 CONFIG_SND_SOC_TEGRA_WM8753=y
-- 
2.20.1

[PATCH v3 1/2] ASoC: tegra20-spdif: remove "default m"

[Michał Mirosław]

Make tegra20-spdif default to N as all other drivers do.

Signed-off-by: Michał Mirosław 
Fixes: 774fec338bfc ("ASoC: Tegra: Implement SPDIF CPU DAI")
---
v3: split-off the defconfig changes
v2: add the symbol to defconfig as suggested by Thierry Reding
---
 sound/soc/tegra/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
index 3d91bd3e59cd..a62cc87551ac 100644
--- a/sound/soc/tegra/Kconfig
+++ b/sound/soc/tegra/Kconfig
@@ -39,7 +39,6 @@ config SND_SOC_TEGRA20_I2S
 config SND_SOC_TEGRA20_SPDIF
tristate "Tegra20 SPDIF interface"
depends on SND_SOC_TEGRA
-   default m
help
  Say Y or M if you want to add support for the Tegra20 SPDIF interface.
  You will also need to select the individual machine drivers to support
-- 
2.20.1

[PATCH v8 1/4] input: elants: document some registers and values

[Michał Mirosław]

Add information found in downstream kernels, to make the code less
magic.

Signed-off-by: Michał Mirosław 
Reviewed-by: Dmitry Osipenko 
Tested-by: Dmitry Osipenko 
---
 drivers/input/touchscreen/elants_i2c.c | 18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/elants_i2c.c 
b/drivers/input/touchscreen/elants_i2c.c
index 50c348297e38..d51cb910fba1 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -82,7 +82,7 @@
 
 #define HEADER_REPORT_10_FINGER0x62
 
-/* Header (4 bytes) plus 3 fill 10-finger packets */
+/* Header (4 bytes) plus 3 full 10-finger packets */
 #define MAX_PACKET_SIZE169
 
 #define BOOT_TIME_DELAY_MS 50
@@ -97,6 +97,10 @@
 #define E_INFO_PHY_SCAN0xD7
 #define E_INFO_PHY_DRIVER  0xD8
 
+/* FW write command, 0x54 0x?? 0x0, 0x01 */
+#define E_POWER_STATE_SLEEP0x50
+#define E_POWER_STATE_RESUME   0x58
+
 #define MAX_RETRIES3
 #define MAX_FW_UPDATE_RETRIES  30
 
@@ -269,8 +273,8 @@ static int elants_i2c_calibrate(struct elants_data *ts)
 {
struct i2c_client *client = ts->client;
int ret, error;
-   static const u8 w_flashkey[] = { 0x54, 0xC0, 0xE1, 0x5A };
-   static const u8 rek[] = { 0x54, 0x29, 0x00, 0x01 };
+   static const u8 w_flashkey[] = { CMD_HEADER_WRITE, 0xC0, 0xE1, 0x5A };
+   static const u8 rek[] = { CMD_HEADER_WRITE, 0x29, 0x00, 0x01 };
static const u8 rek_resp[] = { CMD_HEADER_REK, 0x66, 0x66, 0x66 };
 
disable_irq(client->irq);
@@ -1388,7 +1392,9 @@ static int __maybe_unused elants_i2c_suspend(struct 
device *dev)
 {
struct i2c_client *client = to_i2c_client(dev);
struct elants_data *ts = i2c_get_clientdata(client);
-   const u8 set_sleep_cmd[] = { 0x54, 0x50, 0x00, 0x01 };
+   const u8 set_sleep_cmd[] = {
+   CMD_HEADER_WRITE, E_POWER_STATE_SLEEP, 0x00, 0x01
+   };
int retry_cnt;
int error;
 
@@ -1425,7 +1431,9 @@ static int __maybe_unused elants_i2c_resume(struct device 
*dev)
 {
struct i2c_client *client = to_i2c_client(dev);
struct elants_data *ts = i2c_get_clientdata(client);
-   const u8 set_active_cmd[] = { 0x54, 0x58, 0x00, 0x01 };
+   const u8 set_active_cmd[] = {
+   CMD_HEADER_WRITE, E_POWER_STATE_RESUME, 0x00, 0x01
+   };
int retry_cnt;
int error;
 
-- 
2.20.1

[PATCH v8 2/4] input: elants: support old touch report format

[Michał Mirosław]

Support ELAN touchpad sensor with older firmware as found on eg. Asus
Transformer Pads.

Signed-off-by: Michał Mirosław 
Reviewed-by: Dmitry Osipenko 
Tested-by: Dmitry Osipenko 
---
 drivers/input/touchscreen/elants_i2c.c | 36 ++
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/input/touchscreen/elants_i2c.c 
b/drivers/input/touchscreen/elants_i2c.c
index d51cb910fba1..f1bf3e000e96 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -69,6 +69,7 @@
 #define CMD_HEADER_REK 0x66
 
 /* FW position data */
+#define PACKET_SIZE_OLD40
 #define PACKET_SIZE55
 #define MAX_CONTACT_NUM10
 #define FW_POS_HEADER  0
@@ -853,7 +854,8 @@ static int elants_i2c_fw_update(struct elants_data *ts)
  * Event reporting.
  */
 
-static void elants_i2c_mt_event(struct elants_data *ts, u8 *buf)
+static void elants_i2c_mt_event(struct elants_data *ts, u8 *buf,
+   size_t report_len)
 {
struct input_dev *input = ts->input;
unsigned int n_fingers;
@@ -866,7 +868,8 @@ static void elants_i2c_mt_event(struct elants_data *ts, u8 
*buf)
buf[FW_POS_STATE];
 
dev_dbg(>client->dev,
-   "n_fingers: %u, state: %04x\n",  n_fingers, finger_state);
+   "n_fingers: %u, state: %04x, report_len: %zu\n",
+   n_fingers, finger_state, report_len);
 
/* Note: all fingers have the same tool type */
tool_type = buf[FW_POS_TOOL_TYPE] & BIT(0) ?
@@ -880,8 +883,16 @@ static void elants_i2c_mt_event(struct elants_data *ts, u8 
*buf)
pos = [FW_POS_XY + i * 3];
x = (((u16)pos[0] & 0xf0) << 4) | pos[1];
y = (((u16)pos[0] & 0x0f) << 8) | pos[2];
-   p = buf[FW_POS_PRESSURE + i];
-   w = buf[FW_POS_WIDTH + i];
+   if (report_len == PACKET_SIZE_OLD) {
+   w = buf[FW_POS_WIDTH + i / 2];
+   w >>= 4 * (~i & 1); // little-endian-nibbles
+   w |= w << 4;
+   w |= !w;
+   p = w;
+   } else {
+   p = buf[FW_POS_PRESSURE + i];
+   w = buf[FW_POS_WIDTH + i];
+   }
 
dev_dbg(>client->dev, "i=%d x=%d y=%d p=%d w=%d\n",
i, x, y, p, w);
@@ -913,7 +924,8 @@ static u8 elants_i2c_calculate_checksum(u8 *buf)
return checksum;
 }
 
-static void elants_i2c_event(struct elants_data *ts, u8 *buf)
+static void elants_i2c_event(struct elants_data *ts, u8 *buf,
+size_t report_len)
 {
u8 checksum = elants_i2c_calculate_checksum(buf);
 
@@ -927,7 +939,7 @@ static void elants_i2c_event(struct elants_data *ts, u8 
*buf)
 "%s: unknown packet type: %02x\n",
 __func__, buf[FW_POS_HEADER]);
else
-   elants_i2c_mt_event(ts, buf);
+   elants_i2c_mt_event(ts, buf, report_len);
 }
 
 static irqreturn_t elants_i2c_irq(int irq, void *_dev)
@@ -985,7 +997,8 @@ static irqreturn_t elants_i2c_irq(int irq, void *_dev)
break;
 
case QUEUE_HEADER_SINGLE:
-   elants_i2c_event(ts, >buf[HEADER_SIZE]);
+   elants_i2c_event(ts, >buf[HEADER_SIZE],
+ts->buf[FW_HDR_LENGTH]);
break;
 
case QUEUE_HEADER_NORMAL:
@@ -998,17 +1011,18 @@ static irqreturn_t elants_i2c_irq(int irq, void *_dev)
}
 
report_len = ts->buf[FW_HDR_LENGTH] / report_count;
-   if (report_len != PACKET_SIZE) {
+   if (report_len != PACKET_SIZE &&
+   report_len != PACKET_SIZE_OLD) {
dev_err(>dev,
-   "mismatching report length: %*ph\n",
+   "unsupported report length: %*ph\n",
HEADER_SIZE, ts->buf);
break;
}
 
for (i = 0; i < report_count; i++) {
u8 *buf = ts->buf + HEADER_SIZE +
-   i * PACKET_SIZE;
-   elants_i2c_event(ts, buf);
+ i * report_len;
+   elants_i2c_event(ts, buf, report_len);
}
break;
 
-- 
2.20.1

[PATCH v8 4/4] input: elants: support 0x66 reply opcode for reporting touches

[Michał Mirosław]

From: Dmitry Osipenko 

eKTF3624 touchscreen firmware uses two variants of the reply opcodes for
reporting touch events: one is 0x63 (used by older firmware) and other is
0x66 (used by newer firmware). The 0x66 variant is equal to 0x63 of
eKTH3500, while 0x63 needs small adjustment of the touch pressure value.

Nexus 7 tablet device has eKTF3624 touchscreen and it uses 0x66 opcode for
reporting touch events, let's support it now. Other devices, eg. ASUS TF300T,
use 0x63.

Note: CMD_HEADER_REK is used for replying to calibration requests, it has
the same 0x66 opcode number which eKTF3624 uses for reporting touches.
The calibration replies are handled separately from the the rest of the
commands in the driver by entering into ELAN_WAIT_RECALIBRATION state
and thus this change shouldn't change the old behavior.

Signed-off-by: Dmitry Osipenko 
Tested-by: Michał Mirosław 
Signed-off-by: Michał Mirosław 
---
 drivers/input/touchscreen/elants_i2c.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/elants_i2c.c 
b/drivers/input/touchscreen/elants_i2c.c
index c24d8cdc4251..1cbda6f20d07 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -61,6 +61,15 @@
 #define QUEUE_HEADER_NORMAL0X63
 #define QUEUE_HEADER_WAIT  0x64
 
+/*
+ * Depending on firmware version, eKTF3624 touchscreens may utilize one of
+ * these opcodes for the touch events: 0x63 and 0x66. The 0x63 is used by
+ * older firmware version and differs from 0x66 such that touch pressure
+ * value needs to be adjusted. The 0x66 opcode of newer firmware is equal
+ * to 0x63 of eKTH3500.
+ */
+#define QUEUE_HEADER_NORMAL2   0x66
+
 /* Command header definition */
 #define CMD_HEADER_WRITE   0x54
 #define CMD_HEADER_READ0x53
@@ -1052,7 +1061,6 @@ static irqreturn_t elants_i2c_irq(int irq, void *_dev)
switch (ts->buf[FW_HDR_TYPE]) {
case CMD_HEADER_HELLO:
case CMD_HEADER_RESP:
-   case CMD_HEADER_REK:
break;
 
case QUEUE_HEADER_WAIT:
@@ -1072,6 +1080,7 @@ static irqreturn_t elants_i2c_irq(int irq, void *_dev)
break;
 
case QUEUE_HEADER_NORMAL:
+   case QUEUE_HEADER_NORMAL2:
report_count = ts->buf[FW_HDR_COUNT];
if (report_count == 0 || report_count > 3) {
dev_err(>dev,
-- 
2.20.1

[PATCH v8 0/4] input: elants: Support Asus TF300T and Nexus 7 touchscreens

[Michał Mirosław]

This series cleans up the driver a bit and implements changes needed to
support EKTF3624-based touchscreen used in Asus TF300T, Google Nexus 7
and similar Tegra3-based tablets.

---
v2: extended with Dmitry's patches (replaced v1 patches 3 and 4)
v3: rebased for v5.7-rc1
v4: rebased onto v5.7-rc2+ (current Linus' master)
update "remove unused axes" and "refactor
  elants_i2c_execute_command()" patches after review
add David's patch converting DT binding to YAML
v5: rebased onto dtor/input/for-linus
v6: rebased onto newer dtor/input/for-linus
remove yet unused constants from patch 1
added a new drive-by cleanup (last patch)
v7: rebased onto current dtor/input/for-next
v8: rebased onto current dtor/input/for-linus again
---

Dmitry Osipenko (1):
  input: elants: support 0x66 reply opcode for reporting touches

Michał Mirosław (3):
  input: elants: document some registers and values
  input: elants: support old touch report format
  input: elants: read touchscreen size for EKTF3624

 drivers/input/touchscreen/elants_i2c.c | 149 +
 1 file changed, 127 insertions(+), 22 deletions(-)

-- 
2.20.1

[PATCH v8 3/4] input: elants: read touchscreen size for EKTF3624

[Michał Mirosław]

EKTF3624 as present in Asus TF300T tablet has touchscreen size encoded
in different registers.

Signed-off-by: Michał Mirosław 
Reviewed-by: Dmitry Osipenko 
Tested-by: Dmitry Osipenko 
---
 drivers/input/touchscreen/elants_i2c.c | 84 --
 1 file changed, 79 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/elants_i2c.c 
b/drivers/input/touchscreen/elants_i2c.c
index f1bf3e000e96..c24d8cdc4251 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -35,7 +35,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
@@ -43,6 +43,10 @@
 /* Device, Driver information */
 #define DEVICE_NAME"elants_i2c"
 
+/* Device IDs */
+#define EKTH3500   0
+#define EKTF3624   1
+
 /* Convert from rows or columns into resolution */
 #define ELAN_TS_RESOLUTION(n, m)   (((n) - 1) * (m))
 
@@ -94,6 +98,8 @@
 #define E_ELAN_INFO_REK0xD0
 #define E_ELAN_INFO_TEST_VER   0xE0
 #define E_ELAN_INFO_FW_ID  0xF0
+#define E_INFO_X_RES   0x60
+#define E_INFO_Y_RES   0x63
 #define E_INFO_OSR 0xD6
 #define E_INFO_PHY_SCAN0xD7
 #define E_INFO_PHY_DRIVER  0xD8
@@ -157,6 +163,7 @@ struct elants_data {
 
bool wake_irq_enabled;
bool keep_power_in_suspend;
+   u8 chip_id;
 
/* Must be last to be used for DMA operations */
u8 buf[MAX_PACKET_SIZE] cacheline_aligned;
@@ -434,7 +441,58 @@ static int elants_i2c_query_bc_version(struct elants_data 
*ts)
return 0;
 }
 
-static int elants_i2c_query_ts_info(struct elants_data *ts)
+static int elants_i2c_query_ts_info_ektf(struct elants_data *ts)
+{
+   struct i2c_client *client = ts->client;
+   int error;
+   u8 resp[4];
+   u16 phy_x, phy_y;
+   const u8 get_xres_cmd[] = {
+   CMD_HEADER_READ, E_INFO_X_RES, 0x00, 0x00
+   };
+   const u8 get_yres_cmd[] = {
+   CMD_HEADER_READ, E_INFO_Y_RES, 0x00, 0x00
+   };
+
+   /* Get X/Y size in mm */
+   error = elants_i2c_execute_command(client, get_xres_cmd,
+  sizeof(get_xres_cmd),
+  resp, sizeof(resp), 1,
+  "get X size");
+   if (error)
+   return error;
+
+   phy_x = resp[2] | ((resp[3] & 0xF0) << 4);
+
+   error = elants_i2c_execute_command(client, get_yres_cmd,
+  sizeof(get_yres_cmd),
+  resp, sizeof(resp), 1,
+  "get Y size");
+   if (error)
+   return error;
+
+   phy_y = resp[2] | ((resp[3] & 0xF0) << 4);
+
+   if (!phy_x || !phy_y) {
+   dev_warn(>dev,
+"invalid size data: %d x %d mm\n",
+phy_x, phy_y);
+   return 0;
+   }
+
+   dev_dbg(>dev, "phy_x=%d, phy_y=%d\n", phy_x, phy_y);
+
+   /* calculate resolution from size */
+   ts->x_max = 2240-1;
+   ts->x_res = DIV_ROUND_CLOSEST(ts->prop.max_x, phy_x);
+
+   ts->y_max = 1408-1;
+   ts->y_res = DIV_ROUND_CLOSEST(ts->prop.max_y, phy_y);
+
+   return 0;
+}
+
+static int elants_i2c_query_ts_info_ekth(struct elants_data *ts)
 {
struct i2c_client *client = ts->client;
int error;
@@ -588,8 +646,20 @@ static int elants_i2c_initialize(struct elants_data *ts)
error = elants_i2c_query_fw_version(ts);
if (!error)
error = elants_i2c_query_test_version(ts);
-   if (!error)
-   error = elants_i2c_query_ts_info(ts);
+
+   switch (ts->chip_id) {
+   case EKTH3500:
+   if (!error)
+   error = elants_i2c_query_ts_info_ekth(ts);
+   break;
+   case EKTF3624:
+   if (!error)
+   error = elants_i2c_query_ts_info_ektf(ts);
+   break;
+   default:
+   unreachable();
+   break;
+   }
 
if (error)
ts->iap_mode = ELAN_IAP_RECOVERY;
@@ -1266,6 +1336,9 @@ static int elants_i2c_probe(struct i2c_client *client,
ts->client = client;
i2c_set_clientdata(client, ts);
 
+   if (client->dev.of_node)
+   ts->chip_id = (uintptr_t)of_device_get_match_data(>dev);
+
ts->vcc33 = devm_regulator_get(>dev, "vcc33");
if (IS_ERR(ts->vcc33)) {
error = PTR_ERR(ts->vcc33);
@@ -1495,7 +1568,8 @@ MODULE_DEVICE_TABLE(acpi, elants_acpi_id);
 
 #ifdef CONFIG_OF
 static const struct of_device_id elants_of_match[] = {
-   { .compatible = "elan,ekth3500" },
+   { .compatible = "elan,ekth3500", .data = 

[RESEND v2] ASoC: tegra20-spdif: remove "default m"

[Michał Mirosław]

Make tegra20-spdif default to N as all other drivers do.
Add the selection to defconfigs instead.

Signed-off-by: Michał Mirosław 
Fixes: 774fec338bfc ("ASoC: Tegra: Implement SPDIF CPU DAI")
---
 v2: add the symbol to defconfig as suggested by Thierry Reding
---
 arch/arm/configs/multi_v7_defconfig | 1 +
 arch/arm/configs/tegra_defconfig| 1 +
 sound/soc/tegra/Kconfig | 1 -
 3 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/configs/multi_v7_defconfig 
b/arch/arm/configs/multi_v7_defconfig
index e9e76e32f10f..19342ac738a5 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -743,6 +743,7 @@ CONFIG_SND_SOC_STM32_I2S=m
 CONFIG_SND_SUN4I_CODEC=m
 CONFIG_SND_SOC_TEGRA=m
 CONFIG_SND_SOC_TEGRA20_I2S=m
+CONFIG_SND_SOC_TEGRA20_SPDIF=m
 CONFIG_SND_SOC_TEGRA30_I2S=m
 CONFIG_SND_SOC_TEGRA_RT5640=m
 CONFIG_SND_SOC_TEGRA_WM8753=m
diff --git a/arch/arm/configs/tegra_defconfig b/arch/arm/configs/tegra_defconfig
index fff5fae0db30..08526eb50484 100644
--- a/arch/arm/configs/tegra_defconfig
+++ b/arch/arm/configs/tegra_defconfig
@@ -225,6 +225,7 @@ CONFIG_SND_HDA_CODEC_HDMI=y
 CONFIG_SND_SOC=y
 CONFIG_SND_SOC_TEGRA=y
 CONFIG_SND_SOC_TEGRA20_I2S=y
+CONFIG_SND_SOC_TEGRA20_SPDIF=y
 CONFIG_SND_SOC_TEGRA30_I2S=y
 CONFIG_SND_SOC_TEGRA_RT5640=y
 CONFIG_SND_SOC_TEGRA_WM8753=y
diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
index 3d91bd3e59cd..a62cc87551ac 100644
--- a/sound/soc/tegra/Kconfig
+++ b/sound/soc/tegra/Kconfig
@@ -39,7 +39,6 @@ config SND_SOC_TEGRA20_I2S
 config SND_SOC_TEGRA20_SPDIF
tristate "Tegra20 SPDIF interface"
depends on SND_SOC_TEGRA
-   default m
help
  Say Y or M if you want to add support for the Tegra20 SPDIF interface.
  You will also need to select the individual machine drivers to support
-- 
2.20.1

Re: [BUG] Error applying setting, reverse things back on lot of devices

[Michał Mirosław]

: raw protocol
> > > [4.488691] can: broadcast manager protocol
> > > [4.488705] can: netlink gateway - max_hops=1
> > > [4.488918] Key type dns_resolver registered
> > > [4.489063] Registering SWP/SWPB emulation handler
> > > [4.501808] sun4i-usb-phy 1c13400.phy: Couldn't get regulator 
> > > usb1_vbus... Deferring probe
> > > [4.503853] sun4i-pinctrl 1c20800.pinctrl: supply vcc-pi not found, 
> > > using dummy regulator
> > > [4.505470] sun4i-pinctrl 1c20800.pinctrl: initialized sunXi PIO driver
> > > [4.507391] sun4i-pinctrl 1c20800.pinctrl: supply vcc-pb not found, 
> > > using dummy regulator
> > > [4.508216] printk: console [ttyS0] disabled
> > > [4.528470] 1c28000.serial: ttyS0 at MMIO 0x1c28000 (irq = 45, 
> > > base_baud = 150) is a U6_16550A
> > > [5.585887] printk: console [ttyS0] enabled
> > > [5.590785] dw-apb-uart 1c28c00.serial: Error applying setting, 
> > > reverse things back
> > > [5.609876] sun4i-drm display-engine: bound 110.mixer (ops 
> > > 0xc087f4d0)
> > > [5.618713] sun4i-drm display-engine: bound 120.mixer (ops 
> > > 0xc087f4d0)
> > > [5.626022] sun4i-drm display-engine: bound 1c7.tcon-top (ops 
> > > 0xc08837e0)
> > > [5.633467] sun4i-drm display-engine: bound 1c73000.lcd-controller 
> > > (ops 0xc087b760)
> > > [5.641177] sun8i-dw-hdmi 1ee.hdmi: supply hvcc not found, using 
> > > dummy regulator
> > > [5.682538] sun8i-dw-hdmi 1ee.hdmi: Detected HDMI TX controller 
> > > v1.32a with HDCP (sun8i_dw_hdmi_phy)
> > > [5.692591] sun8i-dw-hdmi 1ee.hdmi: registered DesignWare HDMI I2C 
> > > bus driver
> > > [5.700429] sun4i-drm display-engine: bound 1ee.hdmi (ops 
> > > 0xc087ea28)
> > > [5.707707] [drm] Initialized sun4i-drm 1.0.0 20150629 for 
> > > display-engine on minor 1
> > > [5.715530] sun4i-drm display-engine: [drm] Cannot find any crtc or 
> > > sizes
> > > [5.723563] dwmac-sun8i 1c5.ethernet: Error applying setting, 
> > > reverse things back
> > > [5.734175] sun4i-drm display-engine: [drm] Cannot find any crtc or 
> > > sizes
> > > [5.741110] i2c i2c-1: Not using recovery: no recover_bus() found
> > > [5.748415] axp20x-i2c 1-0034: AXP20x variant AXP221 found
> > > [5.761265] input: axp20x-pek as 
> > > /devices/platform/soc/1c2ac00.i2c/i2c-1/1-0034/axp221-pek/input/input0
> > > [5.780739] vcc-3v3: supplied by regulator-dummy
> > > [5.786045] vdd-cpu: supplied by regulator-dummy
> > > [5.791332] vdd-sys: supplied by regulator-dummy
> > > [5.796585] dcdc4: supplied by regulator-dummy
> > > [5.801647] vcc-dram: supplied by regulator-dummy
> > > [5.806470] vcc-gmac-phy: failed to get the current voltage: -EINVAL
> > > [5.812839] axp20x-regulator axp20x-regulator: Failed to register dc1sw
> > > [5.820291] axp20x-regulator: probe of axp20x-regulator failed with 
> > > error -22
> > 
> > I have just saw thoses 3 lines which are probably the real problem.
> > I have started a new bisect with this error, but it is hitting the same 
> > "crash range" the first one.
> > 
> 
> I have bisected the problem to commit 
> aea6cb99703e17019e025aa71643b4d3e0a24413 ("regulator: resolve supply after 
> creating regulator")
> Reverting this fix my problem.

Can you try the hack below?

Best Regards,
Michał Mirosław

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index a4ffd71696da..9ad091f5f1ab 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1169,6 +1169,9 @@ static int machine_constraints_voltage(struct 
regulator_dev *rdev,
}
 
if (current_uV < 0) {
+   if (current_uV == -EINVAL && rdev->supply_name)
+   return -EPROBE_DEFER;
+
rdev_err(rdev,
 "failed to get the current voltage: %pe\n",
 ERR_PTR(current_uV));

Re: [PATCH v2] iio: adc: exynos: do not rely on 'users' counter in ISR

[Michał Mirosław]

On Mon, Oct 05, 2020 at 09:12:14PM -0700, dmitry.torok...@gmail.com wrote:
> The order in which 'users' counter is decremented vs calling drivers'
> close() method is implementation specific, and we should not rely on
> it. Let's introduce driver private flag and use it to signal ISR
> to exit when device is being closed.
> 
> This has a side-effect of fixing issue of accessing inut->users
> outside of input->mutex protection.
[...]

Reviewed-by: Michał Mirosław 
(after with a fix mentioned below)

> --- a/drivers/iio/adc/exynos_adc.c
> +++ b/drivers/iio/adc/exynos_adc.c
[...]
> @@ -712,6 +715,7 @@ static int exynos_adc_ts_open(struct input_dev *dev)
>  {
>   struct exynos_adc *info = input_get_drvdata(dev);
>  
> + WRITE_ONCE(info->ts_enabled, true);
>   enable_irq(info->tsirq);
>  
>   return 0;
> @@ -721,6 +725,7 @@ static void exynos_adc_ts_close(struct input_dev *dev)
>  {
>   struct exynos_adc *info = input_get_drvdata(dev);
>  
> + WRITE_ONCE(info->ts_enabled, true);
>   disable_irq(info->tsirq);

Shouldn't 'true' be 'false' here?

Best Regards,
Michał Mirosław

Re: [PATCH] iio: adc: exynos: do not rely on 'users' counter in ISR

[Michał Mirosław]

On Mon, Oct 05, 2020 at 10:36:36AM -0700, dmitry.torok...@gmail.com wrote:
> Hi Michał,
> 
> On Mon, Oct 05, 2020 at 01:09:08PM +0200, Michał Mirosław wrote:
> > On Sun, Oct 04, 2020 at 10:24:20PM -0700, dmitry.torok...@gmail.com wrote:
> > > The order in which 'users' counter is decremented vs calling drivers'
> > > close() method is implementation specific, and we should not rely on
> > > it. Let's introduce driver private flag and use it to signal ISR
> > > to exit when device is being closed.
> > > 
> > > This has a side-effect of fixing issue of accessing inut->users
> > > outside of input->mutex protection.
> > > 
> > > Reported-by: Andrzej Pietrasiewicz 
> > > Signed-off-by: Dmitry Torokhov 
> > > ---
> > >  drivers/iio/adc/exynos_adc.c | 8 +++-
> > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
> > > index 22131a677445..7eb2a5df6e98 100644
> > > --- a/drivers/iio/adc/exynos_adc.c
> > > +++ b/drivers/iio/adc/exynos_adc.c
> > > @@ -135,6 +135,8 @@ struct exynos_adc {
> > >   u32 value;
> > >   unsigned intversion;
> > >  
> > > + boolts_enabled;
> > > +
> > >   boolread_ts;
> > >   u32 ts_x;
> > >   u32 ts_y;
> > > @@ -633,7 +635,7 @@ static irqreturn_t exynos_ts_isr(int irq, void 
> > > *dev_id)
> > >   bool pressed;
> > >   int ret;
> > >  
> > > - while (info->input->users) {
> > > + while (info->ts_enabled) {
> > >   ret = exynos_read_s3c64xx_ts(dev, , );
> > >   if (ret == -ETIMEDOUT)
> > >   break;
> > > @@ -712,6 +714,8 @@ static int exynos_adc_ts_open(struct input_dev *dev)
> > >  {
> > >   struct exynos_adc *info = input_get_drvdata(dev);
> > >  
> > > + info->ts_enabled = true;
> > > + mb();
> > >   enable_irq(info->tsirq);
> > >  
> > >   return 0;
> > > @@ -721,6 +725,8 @@ static void exynos_adc_ts_close(struct input_dev *dev)
> > >  {
> > >   struct exynos_adc *info = input_get_drvdata(dev);
> > >  
> > > + info->ts_enabled = false;
> > > + mb();
> > >   disable_irq(info->tsirq);
> > 
> > This should be WRITE_ONCE paired with READ_ONCE in the ISR.
> 
> Why? I can see that maybe full memory barrier is too heavy when we set
> the flag to true, but the only requirement is for the flag to be set
> before we disable IRQ, so any additional guarantees provided by
> WRITE_ONCE are not needed. On the read side we want the flag to be
> noticed eventually, and there is no additional dependencies on the data,
> so it is unclear what READ_ONCE() will give us here.

Without READ_ONCE you have no guarantee that the compiler won't optimize
'while (flag) ...' to 'if (flag) for(;;) ...'.

Maybe the platform has stronger coherency guarantees than Linux memory model
assumes, but if not, a CPU running the ISR (without paired memory barrier)
might not ever see the store from another CPU (both in current and proposed
code).

> > But is the check really needed? I see that this is to break waiting for
> > a touch release event, so I would assume this shouldn't wait forever
> > (unless the hardware is buggy)
> 
> It is not hardware, it is user. Do you want to delay indefinitely
> close() just because user has a finger on the touchscreen?

Ack. This covers the why of loop breaking.

> > and breaking the loop will desync touch
> > state (I would guess this would be noticable by next user).
> Upon next open driver will service the interrupt and provide new set of
> touch coordinates. Userspace is supposed to query current state of
> device when opening it before starting processing events. Or you are
> concerned about some other state?

>From the code I would expect that there is a slight window, wher when the
user releases the touch between close() and open(), the client that open()s
will see a 'pressed' state until the ISR runs again (probably immediately
because of pending interrupt). OTOH, maybe the app should be prepared
for that anyway?

> In any case, this is current driver behavior and if it needs to be
> changed it is a topic for a separate patch.

Agreed.

> 
> Thanks.
> 
> -- 
> Dmitry

Re: [PATCH] iio: adc: exynos: do not rely on 'users' counter in ISR

[Michał Mirosław]

On Sun, Oct 04, 2020 at 10:24:20PM -0700, dmitry.torok...@gmail.com wrote:
> The order in which 'users' counter is decremented vs calling drivers'
> close() method is implementation specific, and we should not rely on
> it. Let's introduce driver private flag and use it to signal ISR
> to exit when device is being closed.
> 
> This has a side-effect of fixing issue of accessing inut->users
> outside of input->mutex protection.
> 
> Reported-by: Andrzej Pietrasiewicz 
> Signed-off-by: Dmitry Torokhov 
> ---
>  drivers/iio/adc/exynos_adc.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
> index 22131a677445..7eb2a5df6e98 100644
> --- a/drivers/iio/adc/exynos_adc.c
> +++ b/drivers/iio/adc/exynos_adc.c
> @@ -135,6 +135,8 @@ struct exynos_adc {
>   u32 value;
>   unsigned intversion;
>  
> + boolts_enabled;
> +
>   boolread_ts;
>   u32 ts_x;
>   u32 ts_y;
> @@ -633,7 +635,7 @@ static irqreturn_t exynos_ts_isr(int irq, void *dev_id)
>   bool pressed;
>   int ret;
>  
> - while (info->input->users) {
> + while (info->ts_enabled) {
>   ret = exynos_read_s3c64xx_ts(dev, , );
>   if (ret == -ETIMEDOUT)
>   break;
> @@ -712,6 +714,8 @@ static int exynos_adc_ts_open(struct input_dev *dev)
>  {
>   struct exynos_adc *info = input_get_drvdata(dev);
>  
> + info->ts_enabled = true;
> + mb();
>   enable_irq(info->tsirq);
>  
>   return 0;
> @@ -721,6 +725,8 @@ static void exynos_adc_ts_close(struct input_dev *dev)
>  {
>   struct exynos_adc *info = input_get_drvdata(dev);
>  
> + info->ts_enabled = false;
> + mb();
>   disable_irq(info->tsirq);

This should be WRITE_ONCE paired with READ_ONCE in the ISR.

But is the check really needed? I see that this is to break waiting for
a touch release event, so I would assume this shouldn't wait forever
(unless the hardware is buggy) and breaking the loop will desync touch
state (I would guess this would be noticable by next user).

Best Regards,
Michał Mirosław

Re: [PATCH v3 10/13] ASoC: tegra: Add audio graph based card driver

[Michał Mirosław]

On Thu, Oct 01, 2020 at 11:03:04PM +0530, Sameer Pujar wrote:
> Add Tegra audio machine driver which is based on generic audio graph card
> driver. It re-uses most of the common stuff from audio graph driver and
> uses the same DT binding. Required Tegra specific customizations are done
> in the driver.
[...]
> + switch (srate) {
> + case 11025:
> + case 22050:
> + case 44100:
> + case 88200:
> + case 176400:
> + plla_out0_rate = chip_data->plla_out0_rates[x11_RATE];
> + plla_rate = chip_data->plla_rates[x11_RATE];
> + break;
> + case 8000:
> + case 16000:
> + case 32000:
> + case 48000:
> + case 96000:
> + case 192000:
[...]

Do you really need to enumerate the frequencies? Wouldn't just checking
srate % 11025 be enough to divide the set in two? Or just calculating
the PLLA base rate by multiplying?

Best Regards,
Michał Mirosław

Re: [PATCH v3 01/13] ASoC: soc-core: Fix component name_prefix parsing

[Michał Mirosław]

On Thu, Oct 01, 2020 at 11:02:55PM +0530, Sameer Pujar wrote:
> The "prefix" can be defined in DAI link node or it can be specified as
> part of the component node itself. Currently "sound-name-prefix" defined
> in a component is not taking effect. Actually the property is not getting
> parsed. It can be fixed by parsing "sound-name-prefix" property whenever
> "prefix" is missing in DAI link Codec node.
[...]
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -1124,7 +1124,8 @@ static void soc_set_name_prefix(struct snd_soc_card 
> *card,
>   for (i = 0; i < card->num_configs; i++) {
>   struct snd_soc_codec_conf *map = >codec_conf[i];
>  
> - if (snd_soc_is_matching_component(>dlc, component)) {
> + if (snd_soc_is_matching_component(>dlc, component) &&
> + map->name_prefix) {
>   component->name_prefix = map->name_prefix;
>   return;
>   }

Hi,

It is not obvious how the patch fixes the problem described. I guess now
map->name_prefix is NULL on some level and overrides prefix found earlier?

Best Regards,
Michał Mirosław

Re: [PATCH v1 2/2] media: tegra-video: Allow building driver with COMPILE_TEST

[Michał Mirosław]

On Tue, Sep 29, 2020 at 08:02:38PM -0700, Sowjanya Komatineni wrote:
> This patch adds COMPILE_TEST option to Kconfig to allow building
> it with COMPILE_TEST option.

Does it build without TEGRA_HOST1X selected? Isn't the previous patch
enough to allow the build with COMPILE_TEST?

Best Regards,
Michał Mirosław

Re: [PATCH v7 0/4] input: elants: Support Asus TF300T and Nexus 7 touchscreen

[Michał Mirosław]

On Sat, Sep 19, 2020 at 11:41:19PM +0200, Michał Mirosław wrote:
> This series cleans up the driver a bit and implements changes needed to
> support EKTF3624-based touchscreen used in Asus TF300T, Google Nexus 7
> and similar Tegra3-based tablets.
[...]

Ping? Should I resend? This got only cosmetic fixups and rebases through
last couple of iterations.

Best Regards
Michał Mirosław

[PATCH v2] ASoC: tegra20-spdif: remove "default m"

[Michał Mirosław]

Make tegra20-spdif default to N as all other drivers do.
Add the selection to defconfigs instead.

Signed-off-by: Michał Mirosław 
Fixes: 774fec338bfc ("ASoC: Tegra: Implement SPDIF CPU DAI")
---
 v2: add the symbol to defconfig as suggested by Thierry Reding
---
 arch/arm/configs/multi_v7_defconfig | 1 +
 arch/arm/configs/tegra_defconfig| 1 +
 sound/soc/tegra/Kconfig | 1 -
 3 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/configs/multi_v7_defconfig 
b/arch/arm/configs/multi_v7_defconfig
index e9e76e32f10f..19342ac738a5 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -743,6 +743,7 @@ CONFIG_SND_SOC_STM32_I2S=m
 CONFIG_SND_SUN4I_CODEC=m
 CONFIG_SND_SOC_TEGRA=m
 CONFIG_SND_SOC_TEGRA20_I2S=m
+CONFIG_SND_SOC_TEGRA20_SPDIF=m
 CONFIG_SND_SOC_TEGRA30_I2S=m
 CONFIG_SND_SOC_TEGRA_RT5640=m
 CONFIG_SND_SOC_TEGRA_WM8753=m
diff --git a/arch/arm/configs/tegra_defconfig b/arch/arm/configs/tegra_defconfig
index fff5fae0db30..08526eb50484 100644
--- a/arch/arm/configs/tegra_defconfig
+++ b/arch/arm/configs/tegra_defconfig
@@ -225,6 +225,7 @@ CONFIG_SND_HDA_CODEC_HDMI=y
 CONFIG_SND_SOC=y
 CONFIG_SND_SOC_TEGRA=y
 CONFIG_SND_SOC_TEGRA20_I2S=y
+CONFIG_SND_SOC_TEGRA20_SPDIF=y
 CONFIG_SND_SOC_TEGRA30_I2S=y
 CONFIG_SND_SOC_TEGRA_RT5640=y
 CONFIG_SND_SOC_TEGRA_WM8753=y
diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
index 3d91bd3e59cd..a62cc87551ac 100644
--- a/sound/soc/tegra/Kconfig
+++ b/sound/soc/tegra/Kconfig
@@ -39,7 +39,6 @@ config SND_SOC_TEGRA20_I2S
 config SND_SOC_TEGRA20_SPDIF
tristate "Tegra20 SPDIF interface"
depends on SND_SOC_TEGRA
-   default m
help
  Say Y or M if you want to add support for the Tegra20 SPDIF interface.
  You will also need to select the individual machine drivers to support
-- 
2.20.1

Re: [PATCH] ASoC: tegra20-spdif: remove "default m"

[Michał Mirosław]

On Mon, Sep 28, 2020 at 10:10:26AM +0200, Thierry Reding wrote:
> On Sat, Sep 26, 2020 at 09:42:40PM +0200, Michał Mirosław wrote:
> > Make tegra20-spdif default to N as all other drivers do.
> > 
> > Signed-off-by: Michał Mirosław 
> > Fixes: 774fec338bfc ("ASoC: Tegra: Implement SPDIF CPU DAI")
> 
> I don't think this is warranted. This doesn't fix a bug or anything.
> It's merely a change in the default configuration. The presence of a
> Fixes: tag is typically used as a hint for people to pick this up into
> stable releases, but I don't think this qualifies.
[...]

Fixes is just for pointing where the bug or issue originated.  I usually
include it to help you -- the reviewer -- and backporters if they ever
want to use this patch. It is not specific to stable-directed patches.

For stable candidates there is 'Cc: stable' tag (no need for this patch).

> So now by default this driver will be disabled, which means that Linux
> is going to regress for people that rely on this driver.

The bug is that this driver (and only this driver in the whole
sound/soc/tegra directory) defaults to m, where all other drivers default
to n (as the policy aboud drivers seems to be [1]). This won't affect
oldconfig, allyesconfig nor allmodconfig, but will not be selected now
for clean builds - meaning less work for those not building for Tegra2.

[1] https://lkml.org/lkml/2017/11/18/257

> You need to at least follow this up with a patch that makes the
> corresponding change in both tegra_defconfig and multi_v7_defconfig to
> ensure that this driver is going to get built by default.

This I can do. Not all such drivers are enabled, though: eg. AHUB driver
is not. Maybe we need bigger refresh of the defconfigs instead?

> Given the above it's probably also a good idea to explain a bit more in
> the commit message about what you're trying to achieve. Yes, "default n"
> is usually the right thing to do and I'm honestly not sure why Stephen
> chose to make this "default m" back in the day. Given that it depends on
> SND_SOC_TEGRA, which itself is "default n", I think this makes some
> sense, even if in retrospect it ended up being a bit inconsistent (you
> could probably argue that all patches after this are the ones that were
> inconsistent instead). This was merged over 9 years ago and a lot of
> common practices have changed over that period of time.

Yes, this is a cleanup. :-)

Best Regards
Michał Mirosław

[PATCH 5/5] mfd: tps65910: remove unused pointers

[Michał Mirosław]

Client pointers in tps65910 data are not used in the drivers.
Remove those fields.

Signed-off-by: Michał Mirosław 
---
 include/linux/mfd/tps65910.h | 5 -
 1 file changed, 5 deletions(-)

diff --git a/include/linux/mfd/tps65910.h b/include/linux/mfd/tps65910.h
index f7398d982f23..701925db75b3 100644
--- a/include/linux/mfd/tps65910.h
+++ b/include/linux/mfd/tps65910.h
@@ -890,11 +890,6 @@ struct tps65910 {
struct regmap *regmap;
unsigned long id;
 
-   /* Client devices */
-   struct tps65910_pmic *pmic;
-   struct tps65910_rtc *rtc;
-   struct tps65910_power *power;
-
/* Device node parsed board data */
struct tps65910_board *of_plat_data;
 
-- 
2.20.1

[PATCH 1/5] gpio: tps65910: use regmap accessors

[Michał Mirosław]

Use regmap accessors directly for register manipulation - removing one
layer of abstraction.

Signed-off-by: Michał Mirosław 
---
 drivers/gpio/gpio-tps65910.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpio/gpio-tps65910.c b/drivers/gpio/gpio-tps65910.c
index 0c785b0fd161..0c0b445c75c0 100644
--- a/drivers/gpio/gpio-tps65910.c
+++ b/drivers/gpio/gpio-tps65910.c
@@ -28,7 +28,7 @@ static int tps65910_gpio_get(struct gpio_chip *gc, unsigned 
offset)
struct tps65910 *tps65910 = tps65910_gpio->tps65910;
unsigned int val;
 
-   tps65910_reg_read(tps65910, TPS65910_GPIO0 + offset, );
+   regmap_read(tps65910->regmap, TPS65910_GPIO0 + offset, );
 
if (val & GPIO_STS_MASK)
return 1;
@@ -43,10 +43,10 @@ static void tps65910_gpio_set(struct gpio_chip *gc, 
unsigned offset,
struct tps65910 *tps65910 = tps65910_gpio->tps65910;
 
if (value)
-   tps65910_reg_set_bits(tps65910, TPS65910_GPIO0 + offset,
+   regmap_set_bits(tps65910->regmap, TPS65910_GPIO0 + offset,
GPIO_SET_MASK);
else
-   tps65910_reg_clear_bits(tps65910, TPS65910_GPIO0 + offset,
+   regmap_clear_bits(tps65910->regmap, TPS65910_GPIO0 + offset,
GPIO_SET_MASK);
 }
 
@@ -59,7 +59,7 @@ static int tps65910_gpio_output(struct gpio_chip *gc, 
unsigned offset,
/* Set the initial value */
tps65910_gpio_set(gc, offset, value);
 
-   return tps65910_reg_set_bits(tps65910, TPS65910_GPIO0 + offset,
+   return regmap_set_bits(tps65910->regmap, TPS65910_GPIO0 + offset,
GPIO_CFG_MASK);
 }
 
@@ -68,7 +68,7 @@ static int tps65910_gpio_input(struct gpio_chip *gc, unsigned 
offset)
struct tps65910_gpio *tps65910_gpio = gpiochip_get_data(gc);
struct tps65910 *tps65910 = tps65910_gpio->tps65910;
 
-   return tps65910_reg_clear_bits(tps65910, TPS65910_GPIO0 + offset,
+   return regmap_clear_bits(tps65910->regmap, TPS65910_GPIO0 + offset,
GPIO_CFG_MASK);
 }
 
@@ -157,7 +157,7 @@ static int tps65910_gpio_probe(struct platform_device *pdev)
if (!pdata->en_gpio_sleep[i])
continue;
 
-   ret = tps65910_reg_set_bits(tps65910,
+   ret = regmap_set_bits(tps65910->regmap,
TPS65910_GPIO0 + i, GPIO_SLEEP_MASK);
if (ret < 0)
dev_warn(tps65910->dev,
-- 
2.20.1

[PATCH 2/5] regulator: tps65910: use regmap accessors

[Michał Mirosław]

Use regmap accessors directly for register manipulation - removing
one layer of abstraction.

Signed-off-by: Michał Mirosław 
---
 drivers/regulator/tps65910-regulator.c | 125 +
 1 file changed, 63 insertions(+), 62 deletions(-)

diff --git a/drivers/regulator/tps65910-regulator.c 
b/drivers/regulator/tps65910-regulator.c
index faa5b3538167..1d5b0a1b86f7 100644
--- a/drivers/regulator/tps65910-regulator.c
+++ b/drivers/regulator/tps65910-regulator.c
@@ -390,8 +390,8 @@ static int tps65911_get_ctrl_register(int id)
 static int tps65910_set_mode(struct regulator_dev *dev, unsigned int mode)
 {
struct tps65910_reg *pmic = rdev_get_drvdata(dev);
-   struct tps65910 *mfd = pmic->mfd;
-   int reg, value, id = rdev_get_id(dev);
+   struct regmap *regmap = rdev_get_regmap(dev);
+   int reg, id = rdev_get_id(dev);
 
reg = pmic->get_ctrl_reg(id);
if (reg < 0)
@@ -399,14 +399,14 @@ static int tps65910_set_mode(struct regulator_dev *dev, 
unsigned int mode)
 
switch (mode) {
case REGULATOR_MODE_NORMAL:
-   return tps65910_reg_update_bits(pmic->mfd, reg,
-   LDO_ST_MODE_BIT | LDO_ST_ON_BIT,
-   LDO_ST_ON_BIT);
+   return regmap_update_bits(regmap, reg,
+ LDO_ST_MODE_BIT | LDO_ST_ON_BIT,
+ LDO_ST_ON_BIT);
case REGULATOR_MODE_IDLE:
-   value = LDO_ST_ON_BIT | LDO_ST_MODE_BIT;
-   return tps65910_reg_set_bits(mfd, reg, value);
+   return regmap_set_bits(regmap, reg,
+  LDO_ST_ON_BIT | LDO_ST_MODE_BIT);
case REGULATOR_MODE_STANDBY:
-   return tps65910_reg_clear_bits(mfd, reg, LDO_ST_ON_BIT);
+   return regmap_clear_bits(regmap, reg, LDO_ST_ON_BIT);
}
 
return -EINVAL;
@@ -415,13 +415,14 @@ static int tps65910_set_mode(struct regulator_dev *dev, 
unsigned int mode)
 static unsigned int tps65910_get_mode(struct regulator_dev *dev)
 {
struct tps65910_reg *pmic = rdev_get_drvdata(dev);
+   struct regmap *regmap = rdev_get_regmap(dev);
int ret, reg, value, id = rdev_get_id(dev);
 
reg = pmic->get_ctrl_reg(id);
if (reg < 0)
return reg;
 
-   ret = tps65910_reg_read(pmic->mfd, reg, );
+   ret = regmap_read(regmap, reg, );
if (ret < 0)
return ret;
 
@@ -435,20 +436,20 @@ static unsigned int tps65910_get_mode(struct 
regulator_dev *dev)
 
 static int tps65910_get_voltage_dcdc_sel(struct regulator_dev *dev)
 {
-   struct tps65910_reg *pmic = rdev_get_drvdata(dev);
+   struct regmap *regmap = rdev_get_regmap(dev);
int ret, id = rdev_get_id(dev);
int opvsel = 0, srvsel = 0, vselmax = 0, mult = 0, sr = 0;
 
switch (id) {
case TPS65910_REG_VDD1:
-   ret = tps65910_reg_read(pmic->mfd, TPS65910_VDD1_OP, );
+   ret = regmap_read(regmap, TPS65910_VDD1_OP, );
if (ret < 0)
return ret;
-   ret = tps65910_reg_read(pmic->mfd, TPS65910_VDD1, );
+   ret = regmap_read(regmap, TPS65910_VDD1, );
if (ret < 0)
return ret;
mult = (mult & VDD1_VGAIN_SEL_MASK) >> VDD1_VGAIN_SEL_SHIFT;
-   ret = tps65910_reg_read(pmic->mfd, TPS65910_VDD1_SR, );
+   ret = regmap_read(regmap, TPS65910_VDD1_SR, );
if (ret < 0)
return ret;
sr = opvsel & VDD1_OP_CMD_MASK;
@@ -457,14 +458,14 @@ static int tps65910_get_voltage_dcdc_sel(struct 
regulator_dev *dev)
vselmax = 75;
break;
case TPS65910_REG_VDD2:
-   ret = tps65910_reg_read(pmic->mfd, TPS65910_VDD2_OP, );
+   ret = regmap_read(regmap, TPS65910_VDD2_OP, );
if (ret < 0)
return ret;
-   ret = tps65910_reg_read(pmic->mfd, TPS65910_VDD2, );
+   ret = regmap_read(regmap, TPS65910_VDD2, );
if (ret < 0)
return ret;
mult = (mult & VDD2_VGAIN_SEL_MASK) >> VDD2_VGAIN_SEL_SHIFT;
-   ret = tps65910_reg_read(pmic->mfd, TPS65910_VDD2_SR, );
+   ret = regmap_read(regmap, TPS65910_VDD2_SR, );
if (ret < 0)
return ret;
sr = opvsel & VDD2_OP_CMD_MASK;
@@ -473,12 +474,10 @@ static int tps65910_get_voltage_dcdc_sel(struct 
regulator_dev *dev)
vselmax = 75;
break;
case TPS65911_REG_VDDCTRL:
-   ret = tps65910_reg_read(pmic->mfd, TPS65911_VDDCTRL_OP,
-   );
+ 

[PATCH 3/5] mfd: tps65911-comparator: use regmap accessors

[Michał Mirosław]

Use regmap accessors directly for register manipulation - removing
one layer of abstraction.

Signed-off-by: Michał Mirosław 
---
 drivers/mfd/tps65911-comparator.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/tps65911-comparator.c 
b/drivers/mfd/tps65911-comparator.c
index 2334cd61c98d..8f4210075913 100644
--- a/drivers/mfd/tps65911-comparator.c
+++ b/drivers/mfd/tps65911-comparator.c
@@ -69,7 +69,7 @@ static int comp_threshold_set(struct tps65910 *tps65910, int 
id, int voltage)
return -EINVAL;
 
val = index << 1;
-   ret = tps65910_reg_write(tps65910, tps_comp.reg, val);
+   ret = regmap_write(tps65910->regmap, tps_comp.reg, val);
 
return ret;
 }
@@ -80,7 +80,7 @@ static int comp_threshold_get(struct tps65910 *tps65910, int 
id)
unsigned int val;
int ret;
 
-   ret = tps65910_reg_read(tps65910, tps_comp.reg, );
+   ret = regmap_read(tps65910->regmap, tps_comp.reg, );
if (ret < 0)
return ret;
 
-- 
2.20.1

[PATCH 4/5] mfd: tps65910: clean up after switching to regmap

[Michał Mirosław]

Remove wrappers around regmap calls to remove now-useless indirection.

Signed-off-by: Michał Mirosław 
---
 drivers/mfd/tps65910.c   | 16 
 include/linux/mfd/tps65910.h | 35 ---
 2 files changed, 8 insertions(+), 43 deletions(-)

diff --git a/drivers/mfd/tps65910.c b/drivers/mfd/tps65910.c
index 11959021b50a..36a52f44cd11 100644
--- a/drivers/mfd/tps65910.c
+++ b/drivers/mfd/tps65910.c
@@ -292,7 +292,7 @@ static int tps65910_ck32k_init(struct tps65910 *tps65910,
if (!pmic_pdata->en_ck32k_xtal)
return 0;
 
-   ret = tps65910_reg_clear_bits(tps65910, TPS65910_DEVCTRL,
+   ret = regmap_clear_bits(tps65910->regmap, TPS65910_DEVCTRL,
DEVCTRL_CK32K_CTRL_MASK);
if (ret < 0) {
dev_err(tps65910->dev, "clear ck32k_ctrl failed: %d\n", ret);
@@ -314,7 +314,7 @@ static int tps65910_sleepinit(struct tps65910 *tps65910,
dev = tps65910->dev;
 
/* enabling SLEEP device state */
-   ret = tps65910_reg_set_bits(tps65910, TPS65910_DEVCTRL,
+   ret = regmap_set_bits(tps65910->regmap, TPS65910_DEVCTRL,
DEVCTRL_DEV_SLP_MASK);
if (ret < 0) {
dev_err(dev, "set dev_slp failed: %d\n", ret);
@@ -322,7 +322,7 @@ static int tps65910_sleepinit(struct tps65910 *tps65910,
}
 
if (pmic_pdata->slp_keepon.therm_keepon) {
-   ret = tps65910_reg_set_bits(tps65910,
+   ret = regmap_set_bits(tps65910->regmap,
TPS65910_SLEEP_KEEP_RES_ON,
SLEEP_KEEP_RES_ON_THERM_KEEPON_MASK);
if (ret < 0) {
@@ -332,7 +332,7 @@ static int tps65910_sleepinit(struct tps65910 *tps65910,
}
 
if (pmic_pdata->slp_keepon.clkout32k_keepon) {
-   ret = tps65910_reg_set_bits(tps65910,
+   ret = regmap_set_bits(tps65910->regmap,
TPS65910_SLEEP_KEEP_RES_ON,
SLEEP_KEEP_RES_ON_CLKOUT32K_KEEPON_MASK);
if (ret < 0) {
@@ -342,7 +342,7 @@ static int tps65910_sleepinit(struct tps65910 *tps65910,
}
 
if (pmic_pdata->slp_keepon.i2chs_keepon) {
-   ret = tps65910_reg_set_bits(tps65910,
+   ret = regmap_set_bits(tps65910->regmap,
TPS65910_SLEEP_KEEP_RES_ON,
SLEEP_KEEP_RES_ON_I2CHS_KEEPON_MASK);
if (ret < 0) {
@@ -354,7 +354,7 @@ static int tps65910_sleepinit(struct tps65910 *tps65910,
return 0;
 
 disable_dev_slp:
-   tps65910_reg_clear_bits(tps65910, TPS65910_DEVCTRL,
+   regmap_clear_bits(tps65910->regmap, TPS65910_DEVCTRL,
DEVCTRL_DEV_SLP_MASK);
 
 err_sleep_init:
@@ -436,11 +436,11 @@ static void tps65910_power_off(void)
 
tps65910 = dev_get_drvdata(_i2c_client->dev);
 
-   if (tps65910_reg_set_bits(tps65910, TPS65910_DEVCTRL,
+   if (regmap_set_bits(tps65910->regmap, TPS65910_DEVCTRL,
DEVCTRL_PWR_OFF_MASK) < 0)
return;
 
-   tps65910_reg_clear_bits(tps65910, TPS65910_DEVCTRL,
+   regmap_clear_bits(tps65910->regmap, TPS65910_DEVCTRL,
DEVCTRL_DEV_ON_MASK);
 }
 
diff --git a/include/linux/mfd/tps65910.h b/include/linux/mfd/tps65910.h
index ce4b9e743f7c..f7398d982f23 100644
--- a/include/linux/mfd/tps65910.h
+++ b/include/linux/mfd/tps65910.h
@@ -913,39 +913,4 @@ static inline int tps65910_chip_id(struct tps65910 
*tps65910)
return tps65910->id;
 }
 
-static inline int tps65910_reg_read(struct tps65910 *tps65910, u8 reg,
-   unsigned int *val)
-{
-   return regmap_read(tps65910->regmap, reg, val);
-}
-
-static inline int tps65910_reg_write(struct tps65910 *tps65910, u8 reg,
-   unsigned int val)
-{
-   return regmap_write(tps65910->regmap, reg, val);
-}
-
-static inline int tps65910_reg_set_bits(struct tps65910 *tps65910, u8 reg,
-   u8 mask)
-{
-   return regmap_update_bits(tps65910->regmap, reg, mask, mask);
-}
-
-static inline int tps65910_reg_clear_bits(struct tps65910 *tps65910, u8 reg,
-   u8 mask)
-{
-   return regmap_update_bits(tps65910->regmap, reg, mask, 0);
-}
-
-static inline int tps65910_reg_update_bits(struct tps65910 *tps65910, u8 reg,
-  u8 mask, u8 val)
-{
-   return regmap_update_bits(tps65910->regmap, reg, mask, val);
-}
-
-static inline int tps65910_irq_get_virq(struct tps65910 *tps65910, int irq)
-{
-   return regmap_irq_get_virq(tps65910->irq_data, irq);
-}
-
 #endif /*  __LINUX_MFD_TPS65910_H */
-- 
2.20.1

[PATCH 0/5] tps65910: cleanup regmap use

[Michał Mirosław]

tps65910 was converted a long time ago to regmap. This series cleans up
after the conversion by removing tps65910_reg_*() indirections and
other unused fields in MFD structure.

Michał Mirosław (5):
  gpio: tps65910: use regmap accessors
  regulator: tps65910: use regmap accessors
  mfd: tps65911-comparator: use regmap accessors
  mfd: tps65910: clean up after switching to regmap
  mfd: tps65910: remove unused pointers

 drivers/gpio/gpio-tps65910.c   |  12 +--
 drivers/mfd/tps65910.c |  16 ++--
 drivers/mfd/tps65911-comparator.c  |   4 +-
 drivers/regulator/tps65910-regulator.c | 125 +
 include/linux/mfd/tps65910.h   |  40 
 5 files changed, 79 insertions(+), 118 deletions(-)

-- 
2.20.1

[PATCH 1/3] regulator: print state at boot

[Michał Mirosław]

Make the initial state of the regulator shown when debugging.

Signed-off-by: Michał Mirosław 
---
 drivers/regulator/core.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 9238613a8c26..bf88d74f5401 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1114,10 +1114,15 @@ static void print_constraints(struct regulator_dev 
*rdev)
if (constraints->valid_modes_mask & REGULATOR_MODE_IDLE)
count += scnprintf(buf + count, len - count, "idle ");
if (constraints->valid_modes_mask & REGULATOR_MODE_STANDBY)
-   count += scnprintf(buf + count, len - count, "standby");
+   count += scnprintf(buf + count, len - count, "standby ");
 
if (!count)
-   scnprintf(buf, len, "no parameters");
+   count = scnprintf(buf, len, "no parameters");
+   else
+   --count;
+
+   count += scnprintf(buf + count, len - count, ", %s",
+   _regulator_is_enabled(rdev) ? "enabled" : "disabled");
 
rdev_dbg(rdev, "%s\n", buf);
 
-- 
2.20.1

[PATCH 0/3] regulator: debugging aids

[Michał Mirosław]

Three simple patches to aid in debugging regulators.

Michał Mirosław (3):
  regulator: print state at boot
  regulator: print symbolic errors in kernel messages
  regulator: resolve supply after creating regulator

 drivers/regulator/core.c | 124 ++-
 1 file changed, 69 insertions(+), 55 deletions(-)

-- 
2.20.1

[PATCH 3/3] regulator: resolve supply after creating regulator

[Michał Mirosław]

When creating a new regulator its supply cannot create the sysfs link
because the device is not yet published. Remove early supply resolving
since it will be done later anyway. This makes the following error
disappear and the symlinks get created instead.

  DCDC_REG1: supplied by VSYS
  VSYS: could not add device link regulator.3 err -2

Note: It doesn't fix the problem for bypassed regulators, though.

Fixes: 45389c47526d ("regulator: core: Add early supply resolution for 
regulators")
Signed-off-by: Michał Mirosław 
---
 drivers/regulator/core.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 6b56a22fd37f..607c2a76bb60 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -5279,15 +5279,20 @@ regulator_register(const struct regulator_desc 
*regulator_desc,
else if (regulator_desc->supply_name)
rdev->supply_name = regulator_desc->supply_name;
 
-   /*
-* Attempt to resolve the regulator supply, if specified,
-* but don't return an error if we fail because we will try
-* to resolve it again later as more regulators are added.
-*/
-   if (regulator_resolve_supply(rdev))
-   rdev_dbg(rdev, "unable to resolve supply\n");
-
ret = set_machine_constraints(rdev, constraints);
+   if (ret == -EPROBE_DEFER) {
+   /* Regulator might be in bypass mode and so needs its supply
+* to set the constraints */
+   /* FIXME: this currently triggers a chicken-and-egg problem
+* when creating -SUPPLY symlink in sysfs to a regulator
+* that is just being created */
+   ret = regulator_resolve_supply(rdev);
+   if (!ret)
+   ret = set_machine_constraints(rdev, constraints);
+   else
+   rdev_dbg(rdev, "unable to resolve supply early: %pe\n",
+ERR_PTR(ret));
+   }
if (ret < 0)
goto wash;
 
-- 
2.20.1

[PATCH 2/3] regulator: print symbolic errors in kernel messages

[Michał Mirosław]

Change all error-printing messages to include error name via %pe instead
of numeric error or nothing.

Signed-off-by: Michał Mirosław 
---
 drivers/regulator/core.c | 94 +---
 1 file changed, 49 insertions(+), 45 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index bf88d74f5401..6b56a22fd37f 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -967,7 +967,8 @@ static int drms_uA_update(struct regulator_dev *rdev)
/* set the optimum mode for our new total regulator load */
err = rdev->desc->ops->set_load(rdev, current_uA);
if (err < 0)
-   rdev_err(rdev, "failed to set load %d\n", current_uA);
+   rdev_err(rdev, "failed to set load %d: %pe\n",
+current_uA, ERR_PTR(err));
} else {
/* get output voltage */
output_uV = regulator_get_voltage_rdev(rdev);
@@ -994,14 +995,15 @@ static int drms_uA_update(struct regulator_dev *rdev)
/* check the new mode is allowed */
err = regulator_mode_constrain(rdev, );
if (err < 0) {
-   rdev_err(rdev, "failed to get optimum mode @ %d uA %d 
-> %d uV\n",
-current_uA, input_uV, output_uV);
+   rdev_err(rdev, "failed to get optimum mode @ %d uA %d 
-> %d uV: %pe\n",
+current_uA, input_uV, output_uV, ERR_PTR(err));
return err;
}
 
err = rdev->desc->ops->set_mode(rdev, mode);
if (err < 0)
-   rdev_err(rdev, "failed to set optimum mode %x\n", mode);
+   rdev_err(rdev, "failed to set optimum mode %x: %pe\n",
+mode, ERR_PTR(err));
}
 
return err;
@@ -1022,14 +1024,14 @@ static int __suspend_set_state(struct regulator_dev 
*rdev,
ret = 0;
 
if (ret < 0) {
-   rdev_err(rdev, "failed to enabled/disable\n");
+   rdev_err(rdev, "failed to enabled/disable: %pe\n", 
ERR_PTR(ret));
return ret;
}
 
if (rdev->desc->ops->set_suspend_voltage && rstate->uV > 0) {
ret = rdev->desc->ops->set_suspend_voltage(rdev, rstate->uV);
if (ret < 0) {
-   rdev_err(rdev, "failed to set voltage\n");
+   rdev_err(rdev, "failed to set voltage: %pe\n", 
ERR_PTR(ret));
return ret;
}
}
@@ -1037,7 +1039,7 @@ static int __suspend_set_state(struct regulator_dev *rdev,
if (rdev->desc->ops->set_suspend_mode && rstate->mode > 0) {
ret = rdev->desc->ops->set_suspend_mode(rdev, rstate->mode);
if (ret < 0) {
-   rdev_err(rdev, "failed to set mode\n");
+   rdev_err(rdev, "failed to set mode: %pe\n", 
ERR_PTR(ret));
return ret;
}
}
@@ -1157,8 +1159,8 @@ static int machine_constraints_voltage(struct 
regulator_dev *rdev,
 
if (current_uV < 0) {
rdev_err(rdev,
-"failed to get the current voltage(%d)\n",
-current_uV);
+"failed to get the current voltage: %pe\n",
+ERR_PTR(current_uV));
return current_uV;
}
 
@@ -1187,8 +1189,8 @@ static int machine_constraints_voltage(struct 
regulator_dev *rdev,
rdev, target_min, target_max);
if (ret < 0) {
rdev_err(rdev,
-   "failed to apply %d-%duV 
constraint(%d)\n",
-   target_min, target_max, ret);
+   "failed to apply %d-%duV constraint: 
%pe\n",
+   target_min, target_max, ERR_PTR(ret));
return ret;
}
}
@@ -1337,7 +1339,7 @@ static int set_machine_constraints(struct regulator_dev 
*rdev,
ret = ops->set_input_current_limit(rdev,
   rdev->constraints->ilim_uA);
if (ret < 0) {
-   rdev_err(rdev, "failed to set input limit\n");
+   rdev_err(rdev, "failed to set input limit:

[PATCH] ASoC: tegra20-spdif: remove "default m"

[Michał Mirosław]

Make tegra20-spdif default to N as all other drivers do.

Signed-off-by: Michał Mirosław 
Fixes: 774fec338bfc ("ASoC: Tegra: Implement SPDIF CPU DAI")
---
 sound/soc/tegra/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
index 3d91bd3e59cd..a62cc87551ac 100644
--- a/sound/soc/tegra/Kconfig
+++ b/sound/soc/tegra/Kconfig
@@ -39,7 +39,6 @@ config SND_SOC_TEGRA20_I2S
 config SND_SOC_TEGRA20_SPDIF
tristate "Tegra20 SPDIF interface"
depends on SND_SOC_TEGRA
-   default m
help
  Say Y or M if you want to add support for the Tegra20 SPDIF interface.
  You will also need to select the individual machine drivers to support
-- 
2.20.1

[PATCH v4 2/2] power: bq25890: support IBAT compensation

[Michał Mirosław]

Add configuration for compensation of IBAT measuring resistor in series
with the battery.

Signed-off-by: Michał Mirosław 
---
v4: renamed properties applying property-suffix
---
 drivers/power/supply/bq25890_charger.c | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/power/supply/bq25890_charger.c 
b/drivers/power/supply/bq25890_charger.c
index 77150667e36b..ab8398f935c5 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -83,6 +83,8 @@ struct bq25890_init_data {
u8 boostf;  /* boost frequency  */
u8 ilim_en; /* enable ILIM pin  */
u8 treg;/* thermal regulation threshold */
+   u8 rbatcomp;/* IBAT sense resistor value*/
+   u8 vclamp;  /* IBAT compensation voltage limit */
 };
 
 struct bq25890_state {
@@ -258,6 +260,8 @@ enum bq25890_table_ids {
TBL_VREG,
TBL_BOOSTV,
TBL_SYSVMIN,
+   TBL_VBATCOMP,
+   TBL_RBATCOMP,
 
/* lookup tables */
TBL_TREG,
@@ -299,6 +303,8 @@ static const union {
[TBL_VREG] ={ .rt = {384, 4608000, 16000} }, /* uV */
[TBL_BOOSTV] =  { .rt = {455, 551, 64000} }, /* uV */
[TBL_SYSVMIN] = { .rt = {300, 370, 10} },/* uV */
+   [TBL_VBATCOMP] ={ .rt = {0,224000, 32000} }, /* uV */
+   [TBL_RBATCOMP] ={ .rt = {0,14, 2} }, /* uOhm */
 
/* lookup tables */
[TBL_TREG] ={ .lt = {bq25890_treg_tbl, BQ25890_TREG_TBL_SIZE} },
@@ -648,7 +654,9 @@ static int bq25890_hw_init(struct bq25890_device *bq)
{F_BOOSTI,   bq->init_data.boosti},
{F_BOOSTF,   bq->init_data.boostf},
{F_EN_ILIM,  bq->init_data.ilim_en},
-   {F_TREG, bq->init_data.treg}
+   {F_TREG, bq->init_data.treg},
+   {F_BATCMP,   bq->init_data.rbatcomp},
+   {F_VCLAMP,   bq->init_data.vclamp},
};
 
ret = bq25890_chip_reset(bq);
@@ -859,11 +867,14 @@ static int bq25890_fw_read_u32_props(struct 
bq25890_device *bq)
{"ti,boost-max-current", false, TBL_BOOSTI, >boosti},
 
/* optional properties */
-   {"ti,thermal-regulation-threshold", true, TBL_TREG, >treg}
+   {"ti,thermal-regulation-threshold", true, TBL_TREG, 
>treg},
+   {"ti,ibatcomp-micro-ohms", true, TBL_RBATCOMP, >rbatcomp},
+   {"ti,ibatcomp-clamp-microvolt", true, TBL_VBATCOMP, 
>vclamp},
};
 
/* initialize data for optional properties */
init->treg = 3; /* 120 degrees Celsius */
+   init->rbatcomp = init->vclamp = 0; /* IBAT compensation disabled */
 
for (i = 0; i < ARRAY_SIZE(props); i++) {
ret = device_property_read_u32(bq->dev, props[i].name,
-- 
2.20.1

[PATCH v4 1/2] power: bq25890: document IBAT compensation DT properties

[Michał Mirosław]

Document new properties for IBAT compensation feature.

Signed-off-by: Michał Mirosław 
---
v2: initial version
v4: renamed properties applying property-suffix
---
 Documentation/devicetree/bindings/power/supply/bq25890.txt | 4 
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/supply/bq25890.txt 
b/Documentation/devicetree/bindings/power/supply/bq25890.txt
index 3b4c69a7fa70..805040c6fff9 100644
--- a/Documentation/devicetree/bindings/power/supply/bq25890.txt
+++ b/Documentation/devicetree/bindings/power/supply/bq25890.txt
@@ -33,6 +33,10 @@ Optional properties:
 - ti,thermal-regulation-threshold: integer, temperature above which the charge
 current is lowered, to avoid overheating (in degrees Celsius). If omitted,
 the default setting will be used (120 degrees);
+- ti,ibatcomp-micro-ohms: integer, value of a resistor in series with
+the battery;
+- ti,ibatcomp-clamp-microvolt: integer, maximum charging voltage adjustment due
+to expected voltage drop on in-series resistor;
 
 Example:
 
-- 
2.20.1

[PATCH v4 0/2] power: bq25890: IBAT compensation support

[Michał Mirosław]

These two patches add support for IBAT compensation feature of bq2589x
chargers.

---
v4 fixed property names for the features and dropped other applied or rejected
   patches

Michał Mirosław (2):
  power: bq25890: document IBAT compensation DT properties
  power: bq25890: support IBAT compensation

 .../devicetree/bindings/power/supply/bq25890.txt  |  4 
 drivers/power/supply/bq25890_charger.c| 15 +--
 2 files changed, 17 insertions(+), 2 deletions(-)

-- 
2.20.1

[PATCH v2] regulator: unexport regulator_lock/unlock()

[Michał Mirosław]

regulator_lock/unlock() was used only to guard
regulator_notifier_call_chain(). As no users remain, make the functions
internal.

Signed-off-by: Michał Mirosław 
---
 drivers/regulator/core.c | 6 ++
 include/linux/regulator/driver.h | 3 ---
 2 files changed, 2 insertions(+), 7 deletions(-)
---
 v2: rebased on current regulator/for-linus

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 7ff507ec875a..8da37e0d1100 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -190,11 +190,10 @@ static inline int regulator_lock_nested(struct 
regulator_dev *rdev,
  * than the one, which initially locked the mutex, it will
  * wait on mutex.
  */
-void regulator_lock(struct regulator_dev *rdev)
+static void regulator_lock(struct regulator_dev *rdev)
 {
regulator_lock_nested(rdev, NULL);
 }
-EXPORT_SYMBOL_GPL(regulator_lock);
 
 /**
  * regulator_unlock - unlock a single regulator
@@ -203,7 +202,7 @@ EXPORT_SYMBOL_GPL(regulator_lock);
  * This function unlocks the mutex when the
  * reference counter reaches 0.
  */
-void regulator_unlock(struct regulator_dev *rdev)
+static void regulator_unlock(struct regulator_dev *rdev)
 {
mutex_lock(_nesting_mutex);
 
@@ -216,7 +215,6 @@ void regulator_unlock(struct regulator_dev *rdev)
 
mutex_unlock(_nesting_mutex);
 }
-EXPORT_SYMBOL_GPL(regulator_unlock);
 
 static bool regulator_supply_is_couple(struct regulator_dev *rdev)
 {
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 8539f34ae42b..11cade73726c 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -533,9 +533,6 @@ int regulator_set_current_limit_regmap(struct regulator_dev 
*rdev,
 int regulator_get_current_limit_regmap(struct regulator_dev *rdev);
 void *regulator_get_init_drvdata(struct regulator_init_data *reg_init_data);
 
-void regulator_lock(struct regulator_dev *rdev);
-void regulator_unlock(struct regulator_dev *rdev);
-
 /*
  * Helper functions intended to be used by regulator drivers prior registering
  * their regulators.
-- 
2.20.1

[PATCH v7 3/4] input: elants: read touchscreen size for EKTF3624

[Michał Mirosław]

EKTF3624 as present in Asus TF300T tablet has touchscreen size encoded
in different registers.

Signed-off-by: Michał Mirosław 
Reviewed-by: Dmitry Osipenko 
Tested-by: Dmitry Osipenko 
---
 drivers/input/touchscreen/elants_i2c.c | 84 --
 1 file changed, 79 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/elants_i2c.c 
b/drivers/input/touchscreen/elants_i2c.c
index f1bf3e000e96..c24d8cdc4251 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -35,7 +35,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
@@ -43,6 +43,10 @@
 /* Device, Driver information */
 #define DEVICE_NAME"elants_i2c"
 
+/* Device IDs */
+#define EKTH3500   0
+#define EKTF3624   1
+
 /* Convert from rows or columns into resolution */
 #define ELAN_TS_RESOLUTION(n, m)   (((n) - 1) * (m))
 
@@ -94,6 +98,8 @@
 #define E_ELAN_INFO_REK0xD0
 #define E_ELAN_INFO_TEST_VER   0xE0
 #define E_ELAN_INFO_FW_ID  0xF0
+#define E_INFO_X_RES   0x60
+#define E_INFO_Y_RES   0x63
 #define E_INFO_OSR 0xD6
 #define E_INFO_PHY_SCAN0xD7
 #define E_INFO_PHY_DRIVER  0xD8
@@ -157,6 +163,7 @@ struct elants_data {
 
bool wake_irq_enabled;
bool keep_power_in_suspend;
+   u8 chip_id;
 
/* Must be last to be used for DMA operations */
u8 buf[MAX_PACKET_SIZE] cacheline_aligned;
@@ -434,7 +441,58 @@ static int elants_i2c_query_bc_version(struct elants_data 
*ts)
return 0;
 }
 
-static int elants_i2c_query_ts_info(struct elants_data *ts)
+static int elants_i2c_query_ts_info_ektf(struct elants_data *ts)
+{
+   struct i2c_client *client = ts->client;
+   int error;
+   u8 resp[4];
+   u16 phy_x, phy_y;
+   const u8 get_xres_cmd[] = {
+   CMD_HEADER_READ, E_INFO_X_RES, 0x00, 0x00
+   };
+   const u8 get_yres_cmd[] = {
+   CMD_HEADER_READ, E_INFO_Y_RES, 0x00, 0x00
+   };
+
+   /* Get X/Y size in mm */
+   error = elants_i2c_execute_command(client, get_xres_cmd,
+  sizeof(get_xres_cmd),
+  resp, sizeof(resp), 1,
+  "get X size");
+   if (error)
+   return error;
+
+   phy_x = resp[2] | ((resp[3] & 0xF0) << 4);
+
+   error = elants_i2c_execute_command(client, get_yres_cmd,
+  sizeof(get_yres_cmd),
+  resp, sizeof(resp), 1,
+  "get Y size");
+   if (error)
+   return error;
+
+   phy_y = resp[2] | ((resp[3] & 0xF0) << 4);
+
+   if (!phy_x || !phy_y) {
+   dev_warn(>dev,
+"invalid size data: %d x %d mm\n",
+phy_x, phy_y);
+   return 0;
+   }
+
+   dev_dbg(>dev, "phy_x=%d, phy_y=%d\n", phy_x, phy_y);
+
+   /* calculate resolution from size */
+   ts->x_max = 2240-1;
+   ts->x_res = DIV_ROUND_CLOSEST(ts->prop.max_x, phy_x);
+
+   ts->y_max = 1408-1;
+   ts->y_res = DIV_ROUND_CLOSEST(ts->prop.max_y, phy_y);
+
+   return 0;
+}
+
+static int elants_i2c_query_ts_info_ekth(struct elants_data *ts)
 {
struct i2c_client *client = ts->client;
int error;
@@ -588,8 +646,20 @@ static int elants_i2c_initialize(struct elants_data *ts)
error = elants_i2c_query_fw_version(ts);
if (!error)
error = elants_i2c_query_test_version(ts);
-   if (!error)
-   error = elants_i2c_query_ts_info(ts);
+
+   switch (ts->chip_id) {
+   case EKTH3500:
+   if (!error)
+   error = elants_i2c_query_ts_info_ekth(ts);
+   break;
+   case EKTF3624:
+   if (!error)
+   error = elants_i2c_query_ts_info_ektf(ts);
+   break;
+   default:
+   unreachable();
+   break;
+   }
 
if (error)
ts->iap_mode = ELAN_IAP_RECOVERY;
@@ -1266,6 +1336,9 @@ static int elants_i2c_probe(struct i2c_client *client,
ts->client = client;
i2c_set_clientdata(client, ts);
 
+   if (client->dev.of_node)
+   ts->chip_id = (uintptr_t)of_device_get_match_data(>dev);
+
ts->vcc33 = devm_regulator_get(>dev, "vcc33");
if (IS_ERR(ts->vcc33)) {
error = PTR_ERR(ts->vcc33);
@@ -1495,7 +1568,8 @@ MODULE_DEVICE_TABLE(acpi, elants_acpi_id);
 
 #ifdef CONFIG_OF
 static const struct of_device_id elants_of_match[] = {
-   { .compatible = "elan,ekth3500" },
+   { .compatible = "elan,ekth3500", .data = 

[PATCH v7 0/4] input: elants: Support Asus TF300T and Nexus 7 touchscreen

[Michał Mirosław]

This series cleans up the driver a bit and implements changes needed to
support EKTF3624-based touchscreen used in Asus TF300T, Google Nexus 7
and similar Tegra3-based tablets.

---
v2: extended with Dmitry's patches (replaced v1 patches 3 and 4)
v3: rebased for v5.7-rc1
v4: rebased onto v5.7-rc2+ (current Linus' master)
update "remove unused axes" and "refactor
  elants_i2c_execute_command()" patches after review
add David's patch converting DT binding to YAML
v5: rebased onto dtor/input/for-linus
v6: rebased onto newer dtor/input/for-linus
remove yet unused constants from patch 1
added a new drive-by cleanup (last patch)
v7: rebased onto current dtor/input/for-next
---

Dmitry Osipenko (1):
  input: elants: support 0x66 reply opcode for reporting touches

Michał Mirosław (3):
  input: elants: document some registers and values
  input: elants: support old touch report format
  input: elants: read touchscreen size for EKTF3624

 drivers/input/touchscreen/elants_i2c.c | 149 +
 1 file changed, 127 insertions(+), 22 deletions(-)

-- 
2.20.1

[PATCH v7 2/4] input: elants: support old touch report format

[Michał Mirosław]

Support ELAN touchpad sensor with older firmware as found on eg. Asus
Transformer Pads.

Signed-off-by: Michał Mirosław 
Reviewed-by: Dmitry Osipenko 
Tested-by: Dmitry Osipenko 
---
 drivers/input/touchscreen/elants_i2c.c | 36 ++
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/input/touchscreen/elants_i2c.c 
b/drivers/input/touchscreen/elants_i2c.c
index d51cb910fba1..f1bf3e000e96 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -69,6 +69,7 @@
 #define CMD_HEADER_REK 0x66
 
 /* FW position data */
+#define PACKET_SIZE_OLD40
 #define PACKET_SIZE55
 #define MAX_CONTACT_NUM10
 #define FW_POS_HEADER  0
@@ -853,7 +854,8 @@ static int elants_i2c_fw_update(struct elants_data *ts)
  * Event reporting.
  */
 
-static void elants_i2c_mt_event(struct elants_data *ts, u8 *buf)
+static void elants_i2c_mt_event(struct elants_data *ts, u8 *buf,
+   size_t report_len)
 {
struct input_dev *input = ts->input;
unsigned int n_fingers;
@@ -866,7 +868,8 @@ static void elants_i2c_mt_event(struct elants_data *ts, u8 
*buf)
buf[FW_POS_STATE];
 
dev_dbg(>client->dev,
-   "n_fingers: %u, state: %04x\n",  n_fingers, finger_state);
+   "n_fingers: %u, state: %04x, report_len: %zu\n",
+   n_fingers, finger_state, report_len);
 
/* Note: all fingers have the same tool type */
tool_type = buf[FW_POS_TOOL_TYPE] & BIT(0) ?
@@ -880,8 +883,16 @@ static void elants_i2c_mt_event(struct elants_data *ts, u8 
*buf)
pos = [FW_POS_XY + i * 3];
x = (((u16)pos[0] & 0xf0) << 4) | pos[1];
y = (((u16)pos[0] & 0x0f) << 8) | pos[2];
-   p = buf[FW_POS_PRESSURE + i];
-   w = buf[FW_POS_WIDTH + i];
+   if (report_len == PACKET_SIZE_OLD) {
+   w = buf[FW_POS_WIDTH + i / 2];
+   w >>= 4 * (~i & 1); // little-endian-nibbles
+   w |= w << 4;
+   w |= !w;
+   p = w;
+   } else {
+   p = buf[FW_POS_PRESSURE + i];
+   w = buf[FW_POS_WIDTH + i];
+   }
 
dev_dbg(>client->dev, "i=%d x=%d y=%d p=%d w=%d\n",
i, x, y, p, w);
@@ -913,7 +924,8 @@ static u8 elants_i2c_calculate_checksum(u8 *buf)
return checksum;
 }
 
-static void elants_i2c_event(struct elants_data *ts, u8 *buf)
+static void elants_i2c_event(struct elants_data *ts, u8 *buf,
+size_t report_len)
 {
u8 checksum = elants_i2c_calculate_checksum(buf);
 
@@ -927,7 +939,7 @@ static void elants_i2c_event(struct elants_data *ts, u8 
*buf)
 "%s: unknown packet type: %02x\n",
 __func__, buf[FW_POS_HEADER]);
else
-   elants_i2c_mt_event(ts, buf);
+   elants_i2c_mt_event(ts, buf, report_len);
 }
 
 static irqreturn_t elants_i2c_irq(int irq, void *_dev)
@@ -985,7 +997,8 @@ static irqreturn_t elants_i2c_irq(int irq, void *_dev)
break;
 
case QUEUE_HEADER_SINGLE:
-   elants_i2c_event(ts, >buf[HEADER_SIZE]);
+   elants_i2c_event(ts, >buf[HEADER_SIZE],
+ts->buf[FW_HDR_LENGTH]);
break;
 
case QUEUE_HEADER_NORMAL:
@@ -998,17 +1011,18 @@ static irqreturn_t elants_i2c_irq(int irq, void *_dev)
}
 
report_len = ts->buf[FW_HDR_LENGTH] / report_count;
-   if (report_len != PACKET_SIZE) {
+   if (report_len != PACKET_SIZE &&
+   report_len != PACKET_SIZE_OLD) {
dev_err(>dev,
-   "mismatching report length: %*ph\n",
+   "unsupported report length: %*ph\n",
HEADER_SIZE, ts->buf);
break;
}
 
for (i = 0; i < report_count; i++) {
u8 *buf = ts->buf + HEADER_SIZE +
-   i * PACKET_SIZE;
-   elants_i2c_event(ts, buf);
+ i * report_len;
+   elants_i2c_event(ts, buf, report_len);
}
break;
 
-- 
2.20.1

[PATCH v7 4/4] input: elants: support 0x66 reply opcode for reporting touches

[Michał Mirosław]

From: Dmitry Osipenko 

eKTF3624 touchscreen firmware uses two variants of the reply opcodes for
reporting touch events: one is 0x63 (used by older firmware) and other is
0x66 (used by newer firmware). The 0x66 variant is equal to 0x63 of
eKTH3500, while 0x63 needs small adjustment of the touch pressure value.

Nexus 7 tablet device has eKTF3624 touchscreen and it uses 0x66 opcode for
reporting touch events, let's support it now. Other devices, eg. ASUS TF300T,
use 0x63.

Note: CMD_HEADER_REK is used for replying to calibration requests, it has
the same 0x66 opcode number which eKTF3624 uses for reporting touches.
The calibration replies are handled separately from the the rest of the
commands in the driver by entering into ELAN_WAIT_RECALIBRATION state
and thus this change shouldn't change the old behavior.

Signed-off-by: Dmitry Osipenko 
Tested-by: Michał Mirosław 
Signed-off-by: Michał Mirosław 
---
 drivers/input/touchscreen/elants_i2c.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/elants_i2c.c 
b/drivers/input/touchscreen/elants_i2c.c
index c24d8cdc4251..1cbda6f20d07 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -61,6 +61,15 @@
 #define QUEUE_HEADER_NORMAL0X63
 #define QUEUE_HEADER_WAIT  0x64
 
+/*
+ * Depending on firmware version, eKTF3624 touchscreens may utilize one of
+ * these opcodes for the touch events: 0x63 and 0x66. The 0x63 is used by
+ * older firmware version and differs from 0x66 such that touch pressure
+ * value needs to be adjusted. The 0x66 opcode of newer firmware is equal
+ * to 0x63 of eKTH3500.
+ */
+#define QUEUE_HEADER_NORMAL2   0x66
+
 /* Command header definition */
 #define CMD_HEADER_WRITE   0x54
 #define CMD_HEADER_READ0x53
@@ -1052,7 +1061,6 @@ static irqreturn_t elants_i2c_irq(int irq, void *_dev)
switch (ts->buf[FW_HDR_TYPE]) {
case CMD_HEADER_HELLO:
case CMD_HEADER_RESP:
-   case CMD_HEADER_REK:
break;
 
case QUEUE_HEADER_WAIT:
@@ -1072,6 +1080,7 @@ static irqreturn_t elants_i2c_irq(int irq, void *_dev)
break;
 
case QUEUE_HEADER_NORMAL:
+   case QUEUE_HEADER_NORMAL2:
report_count = ts->buf[FW_HDR_COUNT];
if (report_count == 0 || report_count > 3) {
dev_err(>dev,
-- 
2.20.1

[PATCH v7 1/4] input: elants: document some registers and values

[Michał Mirosław]

Add information found in downstream kernels, to make the code less
magic.

Signed-off-by: Michał Mirosław 
Reviewed-by: Dmitry Osipenko 
Tested-by: Dmitry Osipenko 
---
 drivers/input/touchscreen/elants_i2c.c | 18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/elants_i2c.c 
b/drivers/input/touchscreen/elants_i2c.c
index 50c348297e38..d51cb910fba1 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -82,7 +82,7 @@
 
 #define HEADER_REPORT_10_FINGER0x62
 
-/* Header (4 bytes) plus 3 fill 10-finger packets */
+/* Header (4 bytes) plus 3 full 10-finger packets */
 #define MAX_PACKET_SIZE169
 
 #define BOOT_TIME_DELAY_MS 50
@@ -97,6 +97,10 @@
 #define E_INFO_PHY_SCAN0xD7
 #define E_INFO_PHY_DRIVER  0xD8
 
+/* FW write command, 0x54 0x?? 0x0, 0x01 */
+#define E_POWER_STATE_SLEEP0x50
+#define E_POWER_STATE_RESUME   0x58
+
 #define MAX_RETRIES3
 #define MAX_FW_UPDATE_RETRIES  30
 
@@ -269,8 +273,8 @@ static int elants_i2c_calibrate(struct elants_data *ts)
 {
struct i2c_client *client = ts->client;
int ret, error;
-   static const u8 w_flashkey[] = { 0x54, 0xC0, 0xE1, 0x5A };
-   static const u8 rek[] = { 0x54, 0x29, 0x00, 0x01 };
+   static const u8 w_flashkey[] = { CMD_HEADER_WRITE, 0xC0, 0xE1, 0x5A };
+   static const u8 rek[] = { CMD_HEADER_WRITE, 0x29, 0x00, 0x01 };
static const u8 rek_resp[] = { CMD_HEADER_REK, 0x66, 0x66, 0x66 };
 
disable_irq(client->irq);
@@ -1388,7 +1392,9 @@ static int __maybe_unused elants_i2c_suspend(struct 
device *dev)
 {
struct i2c_client *client = to_i2c_client(dev);
struct elants_data *ts = i2c_get_clientdata(client);
-   const u8 set_sleep_cmd[] = { 0x54, 0x50, 0x00, 0x01 };
+   const u8 set_sleep_cmd[] = {
+   CMD_HEADER_WRITE, E_POWER_STATE_SLEEP, 0x00, 0x01
+   };
int retry_cnt;
int error;
 
@@ -1425,7 +1431,9 @@ static int __maybe_unused elants_i2c_resume(struct device 
*dev)
 {
struct i2c_client *client = to_i2c_client(dev);
struct elants_data *ts = i2c_get_clientdata(client);
-   const u8 set_active_cmd[] = { 0x54, 0x58, 0x00, 0x01 };
+   const u8 set_active_cmd[] = {
+   CMD_HEADER_WRITE, E_POWER_STATE_RESUME, 0x00, 0x01
+   };
int retry_cnt;
int error;
 
-- 
2.20.1

Re: [PATCH 0/3] regulator: unexport regulator_lock/unlock()

[Michał Mirosław]

On Mon, Sep 07, 2020 at 07:05:49PM +0100, Mark Brown wrote:
> On Mon, 10 Aug 2020 06:33:30 +0200, Michał Mirosław wrote:
> > This removes regulator_lock/unlock() calls around
> > regulator_notifier_call_chain() as they are redundant - drivers
> > already have to guarantee regulator_dev's existence during the call.
> > 
> > This should make reasoing about the lock easier, as this was the only
> > use outside regulator core code.
> > 
> > [...]
> 
> Applied to
> 
>https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 
> for-next
> 
> Thanks!
> 
> [1/3] regulator: don't require mutex for regulator_notifier_call_chain()
>   commit: 3bca239d6184df61a619d78764e0481242d844b4
> [2/3] regulator: remove locking around regulator_notifier_call_chain()
>   commit: e9c142b0d2c08178a9e146d47d8fe397373bda3e
> [3/3] regulator: unexport regulator_lock/unlock()
>   (no commit info)
[...]

It looks like the third one didn't get in? (Can't see it in the
for-next branch).

Best Regards
Michał Mirosław

Re: [PATCH 0/3] regulator: unexport regulator_lock/unlock()

[Michał Mirosław]

On Mon, Aug 10, 2020 at 06:33:30AM +0200, Michał Mirosław wrote:
> This removes regulator_lock/unlock() calls around
> regulator_notifier_call_chain() as they are redundant - drivers
> already have to guarantee regulator_dev's existence during the call.
> 
> This should make reasoing about the lock easier, as this was the only
> use outside regulator core code.
> 
> The only client that needed recursive locking from the notifier chain
> was drivers/usb/host/ohci-da8xx.c, which responds to over-current
> notification by calling regulator_disable().

I can't see the series in regulator/for-next and got no comments.
Should I resend?

As a side note: this is a step towards fixing regulator-coupling-related
locking issues.

Best Regards,
Michał Mirosław

Re: [PATCH v4 00/31] Improvements for Tegra I2C driver

[Michał Mirosław]

On Sat, Sep 05, 2020 at 11:41:20PM +0300, Dmitry Osipenko wrote:
> Hello!
> 
> This series performs refactoring of the Tegra I2C driver code and hardens
> the atomic-transfer mode.
[...]

Pending comments, all LGTM.

Best Regards,
Michał Mirosław

Re: [PATCH v4 30/31] i2c: tegra: Clean up and improve comments

[Michał Mirosław]

On Sun, Sep 06, 2020 at 01:53:56AM +0300, Dmitry Osipenko wrote:
> 06.09.2020 01:47, Michał Mirosław пишет:
> > On Sat, Sep 05, 2020 at 11:41:50PM +0300, Dmitry Osipenko wrote:
> >> Make all comments to be consistent in regards to capitalization and
> >> punctuation, correct spelling and grammar errors.
> > [...]
> >> -  /* Rounds down to not include partial word at the end of buf */
> >> +  /* rounds down to not include partial word at the end of buffer */
> >>words_to_transfer = buf_remaining / BYTES_PER_FIFO_WORD;
> >>  
> >> -  /* It's very common to have < 4 bytes, so optimize that case. */
> >> +  /* it's very common to have < 4 bytes, so optimize that case */
> >>if (words_to_transfer) {
> >>if (words_to_transfer > tx_fifo_avail)
> >>words_to_transfer = tx_fifo_avail;
> >>  
> >>/*
> >> -   * Update state before writing to FIFO.  If this casues us
> >> +   * Update state before writing to FIFO.  If this causes us
> >> * to finish writing all bytes (AKA buf_remaining goes to 0) we
> >> * have a potential for an interrupt (PACKET_XFER_COMPLETE is
> >> * not maskable).  We need to make sure that the isr sees
> >> @@ -800,8 +799,8 @@ static int tegra_i2c_fill_tx_fifo(struct tegra_i2c_dev 
> >> *i2c_dev)
> >>}
> > 
> > Those first letters don't look consistently capitalized. :-)
> 
> In my experience, the more common kernel style is a lowercase for
> single-line comments and the opposite for the multi-line comments.
> Hence, should be good. If you're meaning something else, then please
> clarify.

I don't have a strong opinion about this, but my preference is: If it's
a full sentence, make it look so.

Best Regards,
Michał Mirosław

Re: [PATCH v4 11/31] i2c: tegra: Factor out runtime PM and hardware initialization

[Michał Mirosław]

On Sun, Sep 06, 2020 at 01:24:14AM +0300, Dmitry Osipenko wrote:
> 06.09.2020 01:10, Michał Mirosław пишет:
> > On Sat, Sep 05, 2020 at 11:41:31PM +0300, Dmitry Osipenko wrote:
> >> Factor out runtime PM and hardware initialization into separate function
> >> in order have a cleaner error unwinding in the probe function.
> > [...]
> >> +  ret = tegra_i2c_init_runtime_pm_and_hardware(i2c_dev);
> > [...]
> > 
> > This one doesn't improve the code for me. The problems are: 1) putting two
> > unrelated parts in one function, 2) silently reordered initialization.
> 
> The hardware initialization depends on the resumed RPM and the rest of
> the probe function doesn't care about the RPM. I don't quite understand
> why you're saying that they are unrelated, could you please explain?
> 
> The DMA/RPM initialization is intentionally reordered in order to clean
> up the error handling, like the commit message says. To me it's a clear
> improvement :)

Ok, then wouldn't it be enough to just move this part in the probe()?
A sign of a problem for me is how much information you had to put in
the name of the new function.

Best Regards,
Michał Mirosław