Re: [PATCH v6 01/11] ARM: sunxi: smp: Move assembly code into a file

2018-04-16 Thread Chen-Yu Tsai
On Tue, Apr 17, 2018 at 5:50 AM, Mylène Josserand
 wrote:
> Move the assembly code for cluster cache enabling and resuming
> into an assembly file instead of having it directly in C code.
>
> Remove the CFLAGS because we are using the ARM directive "arch"
> instead.
>
> Signed-off-by: Mylène Josserand 
> ---
>  arch/arm/mach-sunxi/Makefile  |  4 +--
>  arch/arm/mach-sunxi/headsmp.S | 80 +
>  arch/arm/mach-sunxi/mc_smp.c  | 82 
> +++
>  3 files changed, 85 insertions(+), 81 deletions(-)
>  create mode 100644 arch/arm/mach-sunxi/headsmp.S

I'm still not convinced about this whole "move ASM to separate
file" thing, especially now that you aren't actually adding any
sunxi-specific ASM code beyond a simple function call.

Could you drop this for now?

ChenYu


Re: [PATCH v6 03/11] ARM: dts: sun8i: Add R_CPUCFG device node for the A83T dtsi

2018-04-16 Thread Chen-Yu Tsai
On Tue, Apr 17, 2018 at 5:50 AM, Mylène Josserand
 wrote:
> The R_CPUCFG is a collection of registers needed for SMP bringup
> on clusters and cluster's reset.
> For the moment, documentation about this register is found in
> Allwinner's code only.
>
> Signed-off-by: Mylène Josserand 

Reviewed-by: Chen-Yu Tsai 


Re: [PATCH v6 08/11] ARM: sun9i: smp: Add is_sun8i field

2018-04-17 Thread Chen-Yu Tsai
On Tue, Apr 17, 2018 at 3:52 PM, Maxime Ripard
 wrote:
> Hi,
>
> On Mon, Apr 16, 2018 at 11:50:29PM +0200, Mylène Josserand wrote:
>> To prepare the support of sun8i-a83t, add a field in the smp_data
>> structure to know if we are on sun9i-a80 or sun8i-a83t.
>>
>> Add also a global variable to retrieve which architecture we are
>> having.
>>
>> Signed-off-by: Mylène Josserand 
>> ---
>>  arch/arm/mach-sunxi/mc_smp.c | 5 +
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/arm/mach-sunxi/mc_smp.c b/arch/arm/mach-sunxi/mc_smp.c
>> index 03f021d0c73e..9d57ea27dacc 100644
>> --- a/arch/arm/mach-sunxi/mc_smp.c
>> +++ b/arch/arm/mach-sunxi/mc_smp.c
>> @@ -74,6 +74,7 @@ static void __iomem *sram_b_smp_base;
>>
>>  extern void sunxi_mc_smp_secondary_startup(void);
>>  extern void sunxi_mc_smp_resume(void);
>> +static int is_sun8i;
>>
>>  static bool sunxi_core_is_cortex_a15(unsigned int core, unsigned int 
>> cluster)
>>  {
>> @@ -624,6 +625,7 @@ struct sunxi_mc_smp_nodes {
>>  struct sunxi_mc_smp_data {
>>   const char *enable_method;
>>   int (*get_smp_nodes)(struct sunxi_mc_smp_nodes *nodes);
>> + int is_sun8i;
>>  };
>>
>>  static void __init sunxi_mc_smp_put_nodes(struct sunxi_mc_smp_nodes *nodes)
>> @@ -664,6 +666,7 @@ static const struct sunxi_mc_smp_data 
>> sunxi_mc_smp_data[] __initconst = {
>>   {
>>   .enable_method  = "allwinner,sun9i-a80-smp",
>>   .get_smp_nodes  = sun9i_a80_get_smp_nodes,
>> + .is_sun8i   = false,
>
> I'm still not convinced about the name of that flag. sun8i doesn't
> mean anything, really. What you want to discriminate against it what
> you're writing in your commit log: if it is an A80 or an A83t. The
> A33, H3, A23, you name it are also part of the sun8i family, yet they
> are completely irrelevant to this file.
>
> Also, false is the default value, you can leave it out.
>
>>   },
>>  };
>>
>> @@ -697,6 +700,8 @@ static int __init sunxi_mc_smp_init(void)
>>   break;
>>   }
>>
>> + is_sun8i = sunxi_mc_smp_data[i].is_sun8i;
>> +
>
> Do we really need to cache it? Can't we just have a pointer to the SMP
> data structure and use that instead?

I recommended that. We don't need any of the other fields in the SMP
data structure once we're past the init phase. This saves a dereference
or two.

ChenYu


Re: [PATCH v6 01/11] ARM: sunxi: smp: Move assembly code into a file

2018-04-17 Thread Chen-Yu Tsai
On Tue, Apr 17, 2018 at 7:17 PM, Maxime Ripard
 wrote:
> On Tue, Apr 17, 2018 at 11:12:41AM +0800, Chen-Yu Tsai wrote:
>> On Tue, Apr 17, 2018 at 5:50 AM, Mylène Josserand
>>  wrote:
>> > Move the assembly code for cluster cache enabling and resuming
>> > into an assembly file instead of having it directly in C code.
>> >
>> > Remove the CFLAGS because we are using the ARM directive "arch"
>> > instead.
>> >
>> > Signed-off-by: Mylène Josserand 
>> > ---
>> >  arch/arm/mach-sunxi/Makefile  |  4 +--
>> >  arch/arm/mach-sunxi/headsmp.S | 80 
>> > +
>> >  arch/arm/mach-sunxi/mc_smp.c  | 82 
>> > +++
>> >  3 files changed, 85 insertions(+), 81 deletions(-)
>> >  create mode 100644 arch/arm/mach-sunxi/headsmp.S
>>
>> I'm still not convinced about this whole "move ASM to separate
>> file" thing, especially now that you aren't actually adding any
>> sunxi-specific ASM code beyond a simple function call.
>>
>> Could you drop this for now?
>
> I'd really like to have this merged actually. There's a significant
> readibility improvement, so even if there's no particular functional
> improvement, I'd still call it a win.

What parts do you consider hard to read? The extra quotes? Trailing
newline? Or perhaps the __stringify bits?

ChenYu


Re: [PATCH v6 01/11] ARM: sunxi: smp: Move assembly code into a file

2018-04-18 Thread Chen-Yu Tsai
On Wed, Apr 18, 2018 at 4:45 PM, Maxime Ripard
 wrote:
> On Tue, Apr 17, 2018 at 07:25:15PM +0800, Chen-Yu Tsai wrote:
>> On Tue, Apr 17, 2018 at 7:17 PM, Maxime Ripard
>>  wrote:
>> > On Tue, Apr 17, 2018 at 11:12:41AM +0800, Chen-Yu Tsai wrote:
>> >> On Tue, Apr 17, 2018 at 5:50 AM, Mylène Josserand
>> >>  wrote:
>> >> > Move the assembly code for cluster cache enabling and resuming
>> >> > into an assembly file instead of having it directly in C code.
>> >> >
>> >> > Remove the CFLAGS because we are using the ARM directive "arch"
>> >> > instead.
>> >> >
>> >> > Signed-off-by: Mylène Josserand 
>> >> > ---
>> >> >  arch/arm/mach-sunxi/Makefile  |  4 +--
>> >> >  arch/arm/mach-sunxi/headsmp.S | 80 
>> >> > +
>> >> >  arch/arm/mach-sunxi/mc_smp.c  | 82 
>> >> > +++
>> >> >  3 files changed, 85 insertions(+), 81 deletions(-)
>> >> >  create mode 100644 arch/arm/mach-sunxi/headsmp.S
>> >>
>> >> I'm still not convinced about this whole "move ASM to separate
>> >> file" thing, especially now that you aren't actually adding any
>> >> sunxi-specific ASM code beyond a simple function call.
>> >>
>> >> Could you drop this for now?
>> >
>> > I'd really like to have this merged actually. There's a significant
>> > readibility improvement, so even if there's no particular functional
>> > improvement, I'd still call it a win.
>>
>> What parts do you consider hard to read? The extra quotes? Trailing
>> newline? Or perhaps the __stringify bits?
>
> All of this, plus the clobbers and operands.

Ok. Lets move it then.

The kbuild reports indicate this still needs some work though.

ChenYu


Re: [PATCH] extcon: Split out extcon header file for consumer and provider device

2017-10-02 Thread Chen-Yu Tsai
On Fri, Sep 29, 2017 at 8:01 AM, Chanwoo Choi  wrote:
> The extcon has two type of extcon devices as following.
> - 'extcon provider deivce' adds new extcon device and detect the
>state/properties of external connector. Also, it notifies the
>state/properties to the extcon consumer device.
> - 'extcon consumer device' gets the change state/properties
>from extcon provider device.
> Prior to that, include/linux/extcon.h contains all exported API for
> both provider and consumer device driver. To clarify the meaning of
> header file and to remove the wrong use-case on consumer device,
> this patch separates into extcon.h and extcon-provider.h.
>
> [Description for include/linux/{extcon.h|extcon-provider.h}]
> - extcon.h includes the extcon API and data structure for extcon consumer
>   device driver. This header file contains the following APIs:
>   : Register/unregister the notifier to catch the change of extcon device
>   : Get the extcon device instance
>   : Get the extcon device name
>   : Get the state of each external connector
>   : Get the property value of each external connector
>   : Get the property capability of each external connector
>
> - extcon-provider.h includes the extcon API and data structure for extcon
>   provider device driver. This header file contains the following APIs:
>   : Include 'include/linux/extcon.h'
>   : Allocate the memory for extcon device instance
>   : Register/unregister extcon device
>   : Set the state of each external connector
>   : Set the property value of each external connector
>   : Set the property capability of each external connector
>
> Cc: Felipe Balbi 
> Cc: Kishon Vijay Abraham I 
> Cc: Greg Kroah-Hartman 
> Cc: Sebastian Reichel 
> Cc: Lee Jones 
> Signed-off-by: Chanwoo Choi 
> ---

For

>  drivers/extcon/extcon-axp288.c|   2 +-

and

>  drivers/phy/allwinner/phy-sun4i-usb.c |   2 +-

Acked-by: Chen-Yu Tsai 


Re: [PATCH 4/4] drm/sun4i: make sure we don't have a commit pending

2017-07-14 Thread Chen-Yu Tsai
Hi,

On Thu, Jul 13, 2017 at 10:41 PM, Maxime Ripard
 wrote:
> In the earlier display engine designs, any register access while a commit
> is pending is forbidden.
>
> One of the symptoms is that reading a register will return another, random,
> register value which can lead to register corruptions if we ever do a
> read/modify/write cycle.

Alternatively, if changes to the backend (layers) are guaranteed to happen
while the CRTC is disabled (which seems to be the case after looking at
drm_atomic_helper_commit_planes and drm_atomic_helper_commit_tail), we
could just turn on register auto-commit all the time and not deal with
this.

ChenYu

>
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/gpu/drm/sun4i/sun4i_backend.c | 14 ++
>  drivers/gpu/drm/sun4i/sun4i_crtc.c|  1 +
>  2 files changed, 15 insertions(+)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c 
> b/drivers/gpu/drm/sun4i/sun4i_backend.c
> index cf480218daa5..ce1f40f7511e 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> @@ -67,6 +67,19 @@ static void sun4i_backend_commit(struct sunxi_engine 
> *engine)
>  SUN4I_BACKEND_REGBUFFCTL_LOADCTL);
>  }
>
> +static int sun4i_backend_commit_poll(struct sunxi_engine *engine)
> +{
> +   u32 val;
> +
> +   DRM_DEBUG_DRIVER("Polling for the commit to end\n");
> +
> +   return regmap_read_poll_timeout(engine->regs,
> +   SUN4I_BACKEND_REGBUFFCTL_REG,
> +   val,
> +   !(val & 
> SUN4I_BACKEND_REGBUFFCTL_LOADCTL),
> +   100, 5);
> +}
> +
>  void sun4i_backend_layer_enable(struct sun4i_backend *backend,
> int layer, bool enable)
>  {
> @@ -330,6 +343,7 @@ static int sun4i_backend_of_get_id(struct device_node 
> *node)
>
>  static const struct sunxi_engine_ops sun4i_backend_engine_ops = {
> .commit = sun4i_backend_commit,
> +   .commit_poll= sun4i_backend_commit_poll,
> .layers_init= sun4i_layers_init,
> .apply_color_correction = 
> sun4i_backend_apply_color_correction,
> .disable_color_correction   = 
> sun4i_backend_disable_color_correction,
> diff --git a/drivers/gpu/drm/sun4i/sun4i_crtc.c 
> b/drivers/gpu/drm/sun4i/sun4i_crtc.c
> index 2eba1d8639d8..31550493fa1d 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_crtc.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_crtc.c
> @@ -34,6 +34,7 @@ static void sun4i_crtc_atomic_begin(struct drm_crtc *crtc,
> struct drm_crtc_state *old_state)
>  {
> struct sun4i_crtc *scrtc = drm_crtc_to_sun4i_crtc(crtc);
> +   struct sunxi_engine *engine = scrtc->engine;
> struct drm_device *dev = crtc->dev;
> unsigned long flags;
>
> --
> git-series 0.9.1


Re: [PATCH 4/4] drm/sun4i: make sure we don't have a commit pending

2017-07-16 Thread Chen-Yu Tsai
On Mon, Jul 17, 2017 at 2:55 PM, Maxime Ripard
 wrote:
> On Fri, Jul 14, 2017 at 04:56:01PM +0800, Chen-Yu Tsai wrote:
>> Hi,
>>
>> On Thu, Jul 13, 2017 at 10:41 PM, Maxime Ripard
>>  wrote:
>> > In the earlier display engine designs, any register access while a commit
>> > is pending is forbidden.
>> >
>> > One of the symptoms is that reading a register will return another, random,
>> > register value which can lead to register corruptions if we ever do a
>> > read/modify/write cycle.
>>
>> Alternatively, if changes to the backend (layers) are guaranteed to happen
>> while the CRTC is disabled (which seems to be the case after looking at
>> drm_atomic_helper_commit_planes and drm_atomic_helper_commit_tail), we
>> could just turn on register auto-commit all the time and not deal with
>> this.
>
> As far as I understand, it will only be the case if we need a new
> modeset or we changed the active CRTC or connectors. But if you change
> only the format, buffers or properties it won't be the case, and we'll
> need to commit.

So in other words, if someone were to use it for actual compositing and
moved the upper composited layer around, we would need commit support to be
safe.

Sounds more or less like something a video player would do.

Thanks
ChenYu


Re: [PATCH v2 2/2] phy: sun4i-usb: Replace the deprecated extcon API

2017-01-04 Thread Chen-Yu Tsai
On Fri, Dec 30, 2016 at 12:11 PM, Chanwoo Choi  wrote:
> This patch replaces the deprecated extcon API as following:
> - extcon_set_cable_state_() -> extcon_set_state_sync()
>
> Cc: Kishon Vijay Abraham I 
> Cc: Maxime Ripard 
> Cc: Chen-Yu Tsai 
> Signed-off-by: Chanwoo Choi 

Acked-by: Chen-Yu Tsai 

> ---
>  drivers/phy/phy-sun4i-usb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
> index bf28a0fdd569..eae171e18043 100644
> --- a/drivers/phy/phy-sun4i-usb.c
> +++ b/drivers/phy/phy-sun4i-usb.c
> @@ -534,7 +534,7 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct 
> work_struct *work)
> mutex_unlock(&phy0->mutex);
>
> if (id_notify) {
> -   extcon_set_cable_state_(data->extcon, EXTCON_USB_HOST,
> +   extcon_set_state_sync(data->extcon, EXTCON_USB_HOST,
> !id_det);
> /* When leaving host mode force end the session here */
> if (force_session_end && id_det == 1) {
> @@ -547,7 +547,7 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct 
> work_struct *work)
> }
>
> if (vbus_notify)
> -   extcon_set_cable_state_(data->extcon, EXTCON_USB, vbus_det);
> +   extcon_set_state_sync(data->extcon, EXTCON_USB, vbus_det);
>
> if (sun4i_usb_phy0_poll(data))
> queue_delayed_work(system_wq, &data->detect, POLL_TIME);
> --
> 1.9.1
>


Re: [alsa-devel] [PATCH 000/159] ASoC: codec cleanup - codec duplicate functions

2016-08-08 Thread Chen-Yu Tsai
Hi,

On Mon, Aug 8, 2016 at 4:45 PM, Kuninori Morimoto
 wrote:
>
> Hi Mark
>
> These are codec duplicate functions cleanup patches - v1
> We would like to switch to "component" style base ASoC in the future
> instead of current "CPU/Codec/Platform/Card" style.
>
> Current "component" already has callback functions, and "codec" is based
> on "component", but "codec" is still requesting/using its original callback.
> This is bacause of historical reason, but this is the time to cleanup.
>
> Lars's opinion is that this kind of cleanup can be final stage,
> but I think these are needed for "component" base style switching
>
> These are very big patch-set, but it doesn't add new features.
> [001/159] - [158/159] move callback functions from codec to component,
> and last [159/159] will remove codec side callback.

This series would probably be better for others to look through if the
codec name came after "ASoC: ", instead of at the end of the subject,
where it gets pushed way beyond 80 characters due to the "[alsa-devel]"
and "[PATCH xxx/yyy]" tags.


Regards
ChenYu

> We need this kind of cleanup for .probe/.remove too, and on Platform too.
> This cares codec duplicate callback function only as 1st step.
> I will post codec .probe/.remove cleanup if these are accepted,
> and will post platform side cleanup too.
>
> I build-tested these patches with make allyesconfig on x86,
> and tested on Lager board.
>
> Best regards
> ---
> Kuninori Morimoto
> ___
> Alsa-devel mailing list
> alsa-de...@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel