Re: [RFC v2 0/4] Add basic support for ASV

2013-11-17 Thread Sachin Kamat
Hi MyungJoo,

On 18 November 2013 08:07, MyungJoo Ham  wrote:
>
> 2013. 11. 15. 오후 8:44에 "Sachin Kamat" 님이 작성:
>
>
>>
>> Original cover letter from Yadwinder:
>> This series is to add basic common infrastructure for ASV.
>>  Basically ASV is a technique used on samsung SoCs, which provides the
>> recommended supply voltage for dvfs of arm, mif etc. For a given operating
>> frequency, the voltage is recommended based on SoC's ASV group.
>> ASV group gets fussed on SoCs during process of mass production.
>>
>> This series includes:
>>  - basic common infrastructue for ASV. It provides common APIs for user
>> drivers
>> like cpufreq & devfreq and and an interface for SoC specific drivers to
>> register ASV members(instances)
>>  - a common platform driver to register ASV members for exynos SoCs
>>  - an example providing minimal support (only for ARM ASV) for exynos5250
>> chips
>>
>> Its just basic skelton which I wanted to get it reviewed or discussed in
>> early stage, before going ahead on further development based on it.
>>  Presently example is based on static ASV table provided in SoC specific
>> file,
>> which I expects to go into DT. But exactly how and where needs to be
>> discussed,
>> may be in next revisions once we get through the basic skelton.
>>  Also the location of driver in kernel may also seem odd to someone and
>> many more things :).
>>
>> Looking for your valuable reviews and suggestions.
>
> Hi,
>
> As I have commented on the previous thread of Yadwinder, please share the
> directory woth AVS, which is conceptually a superset of ASV.
>
> Ultimately, it would be best if you can supply an infra that can be shared
> with the current AVS, which does not have any currently.
>

Yadwinder has already replied to your suggestion about his concerns. Please
share your comments.

-- 
With warm regards,
Sachin
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/3] thermal: samsung: Clean up and add support for Exynos5420

2013-11-17 Thread Naveen Krishna Ch
Hello All,

On 12 November 2013 12:05, Naveen Krishna Chatradhi
 wrote:
> This patchset does a little clean up of the existing code
> 1.  [v9] thermal: samsung: replace inten_ bit fields with intclr_
> 2.  [v9] thermal: samsung: change base_common to more meaningful base_second
>
> adds support for Exynos5420 in the driver and
> 3.  [v9] thermal: samsung: Add TMU support for Exynos5420 SoCs
> also adds the device tree nodes for the same to exynos5420.dtsi 
> (linux-samsung.git)
> 4.  [v3] ARM: dts: Exynos5420: Add device nodes for TMU blocks
>
> Naveen Krishna Chatradhi (3):
>   thermal: samsung: replace inten_ bit fields with intclr_
>   thermal: samsung: change base_common to more meaningful base_second
>   thermal: samsung: Add TMU support for Exynos5420 SoCs
>   ARM: dts: Exynos5420: Add device nodes for TMU blocks
>
>  .../devicetree/bindings/thermal/exynos-thermal.txt |   49 +++-
>  drivers/thermal/samsung/exynos_tmu.c   |   78 +---
>  drivers/thermal/samsung/exynos_tmu.h   |   22 ++--
>  drivers/thermal/samsung/exynos_tmu_data.c  |  126 
> ++--
>  drivers/thermal/samsung/exynos_tmu_data.h  |   12 +-
>  5 files changed, 249 insertions(+), 38 deletions(-)
>
> --
> 1.7.10.4
Any comments on this patch set please.
>



-- 
Shine bright,
(: Nav :)
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4 v9] thermal: samsung: replace inten_ bit fields with intclr_

2013-11-17 Thread Naveen Krishna Ch
Hello All,

On 12 November 2013 12:06, Naveen Krishna Chatradhi
 wrote:
> This patch replaces the inten_rise_shift/mask and inten_fall_shift/mask
> with intclr_rise_shift/mask and intclr_fall_shift/mask respectively.
> Currently, inten_rise_shift/mask and inten_fall_shift/mask bits are only used
> to configure intclr related registers.
>
> Description of H/W:
> The offset for the bits in the CLEAR register are not consistent across TMU
> modules in Exynso5250, 5420 and 5440.
>
> On Exynos5250, the FALL interrupt related en, status and clear bits are
> available at an offset of
> 16 in INTEN, INTSTAT registers and at an offset of
> 12 in INTCLEAR register.
>
> On Exynos5420, the FALL interrupt related en, status and clear bits are
> available at an offset of
> 16 in INTEN, INTSTAT and INTCLEAR registers.
>
> On Exynos5440,
> the FALL_IRQEN bits are at an offset of 4
> and the RISE_IRQEN bits are at an offset of 0
>
> Signed-off-by: Naveen Krishna Chatradhi 
> ---
> Changes since v8:
> 1. Modified the patch description,
> 2. replaces the inten_rise/fall_shift/mask with intclr_rise/fall_shift/mask
>
>  drivers/thermal/samsung/exynos_tmu.c  |6 +++---
>  drivers/thermal/samsung/exynos_tmu.h  |   16 
>  drivers/thermal/samsung/exynos_tmu_data.c |   18 +-
>  drivers/thermal/samsung/exynos_tmu_data.h |4 ++--
>  4 files changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/thermal/samsung/exynos_tmu.c 
> b/drivers/thermal/samsung/exynos_tmu.c
> index 32f38b9..c493245 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -237,7 +237,7 @@ skip_calib_data:
> writeb(pdata->trigger_levels[i], data->base +
> reg->threshold_th0 + i * sizeof(reg->threshold_th0));
>
> -   writel(reg->inten_rise_mask, data->base + reg->tmu_intclear);
> +   writel(reg->intclr_rise_mask, data->base + reg->tmu_intclear);
> } else {
> /* Write temperature code for rising and falling threshold */
> for (i = 0;
> @@ -264,8 +264,8 @@ skip_calib_data:
> writel(falling_threshold,
> data->base + reg->threshold_th1);
>
> -   writel((reg->inten_rise_mask << reg->inten_rise_shift) |
> -   (reg->inten_fall_mask << reg->inten_fall_shift),
> +   writel((reg->intclr_rise_mask << reg->intclr_rise_shift) |
> +   (reg->intclr_fall_mask << reg->intclr_fall_shift),
> data->base + reg->tmu_intclear);
>
> /* if last threshold limit is also present */
> diff --git a/drivers/thermal/samsung/exynos_tmu.h 
> b/drivers/thermal/samsung/exynos_tmu.h
> index 3fb6554..980859a 100644
> --- a/drivers/thermal/samsung/exynos_tmu.h
> +++ b/drivers/thermal/samsung/exynos_tmu.h
> @@ -122,10 +122,6 @@ enum soc_type {
>   * @threshold_th3_l0_shift: shift bits of level0 threshold temperature.
>   * @tmu_inten: register containing the different threshold interrupt
> enable bits.
> - * @inten_rise_shift: shift bits of all rising interrupt bits.
> - * @inten_rise_mask: mask bits of all rising interrupt bits.
> - * @inten_fall_shift: shift bits of all rising interrupt bits.
> - * @inten_fall_mask: mask bits of all rising interrupt bits.
>   * @inten_rise0_shift: shift bits of rising 0 interrupt bits.
>   * @inten_rise1_shift: shift bits of rising 1 interrupt bits.
>   * @inten_rise2_shift: shift bits of rising 2 interrupt bits.
> @@ -136,6 +132,10 @@ enum soc_type {
>   * @inten_fall3_shift: shift bits of falling 3 interrupt bits.
>   * @tmu_intstat: Register containing the interrupt status values.
>   * @tmu_intclear: Register for clearing the raised interrupt status.
> + * @intclr_fall_shift: shift bits for interrupt clear fall 0
> + * @intclr_rise_shift: shift bits of all rising interrupt bits.
> + * @intclr_rise_mask: mask bits of all rising interrupt bits.
> + * @intclr_fall_mask: mask bits of all rising interrupt bits.
>   * @emul_con: TMU emulation controller register.
>   * @emul_temp_shift: shift bits of emulation temperature.
>   * @emul_time_shift: shift bits of emulation time.
> @@ -191,10 +191,6 @@ struct exynos_tmu_registers {
> u32 threshold_th3_l0_shift;
>
> u32 tmu_inten;
> -   u32 inten_rise_shift;
> -   u32 inten_rise_mask;
> -   u32 inten_fall_shift;
> -   u32 inten_fall_mask;
> u32 inten_rise0_shift;
> u32 inten_rise1_shift;
> u32 inten_rise2_shift;
> @@ -207,6 +203,10 @@ struct exynos_tmu_registers {
> u32 tmu_intstat;
>
> u32 tmu_intclear;
> +   u32 intclr_fall_shift;
> +   u32 intclr_rise_shift;
> +   u32 intclr_fall_mask;
> +   u32 intclr_rise_mask;
>
> u32 emul_con;
> u32 emul_temp_shift;
> diff --git a/drivers

Re: [PATCH 2/4 v9] thermal: samsung: change base_common to more meaningful base_second

2013-11-17 Thread Naveen Krishna Ch
Hello All,

On 12 November 2013 12:06, Naveen Krishna Chatradhi
 wrote:
> On Exynos5440 and Exynos5420 there are registers common
> across the TMU channels.
>
> To support that, we introduced a ADDRESS_MULTIPLE flag in the
> driver and the 2nd set of register base and size are provided
> in the "reg" property of the node.
>
> As per Amit's suggestion, this patch changes the base_common
> to base_second and SHARED_MEMORY to ADDRESS_MULTIPLE.
>
> Signed-off-by: Naveen Krishna Chatradhi 
> ---
> Changes since v8:
>  None
>  .../devicetree/bindings/thermal/exynos-thermal.txt   |4 ++--
>  drivers/thermal/samsung/exynos_tmu.c |   14 
> +++---
>  drivers/thermal/samsung/exynos_tmu.h |4 ++--
>  drivers/thermal/samsung/exynos_tmu_data.c|2 +-
>  4 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/thermal/exynos-thermal.txt 
> b/Documentation/devicetree/bindings/thermal/exynos-thermal.txt
> index 284f530..116cca0 100644
> --- a/Documentation/devicetree/bindings/thermal/exynos-thermal.txt
> +++ b/Documentation/devicetree/bindings/thermal/exynos-thermal.txt
> @@ -11,8 +11,8 @@
>  - reg : Address range of the thermal registers. For soc's which has multiple
> instances of TMU and some registers are shared across all TMU's like
> interrupt related then 2 set of register has to supplied. First set
> -   belongs to each instance of TMU and second set belongs to common TMU
> -   registers.
> +   belongs to each instance of TMU and second set belongs to second set
> +   of common TMU registers.
>  - interrupts : Should contain interrupt for thermal system
>  - clocks : The main clock for TMU device
>  - clock-names : Thermal system clock name
> diff --git a/drivers/thermal/samsung/exynos_tmu.c 
> b/drivers/thermal/samsung/exynos_tmu.c
> index c493245..bbd0fc3 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -41,7 +41,7 @@
>   * @id: identifier of the one instance of the TMU controller.
>   * @pdata: pointer to the tmu platform/configuration data
>   * @base: base address of the single instance of the TMU controller.
> - * @base_common: base address of the common registers of the TMU controller.
> + * @base_second: base address of the common registers of the TMU controller.
>   * @irq: irq number of the TMU controller.
>   * @soc: id of the SOC type.
>   * @irq_work: pointer to the irq work structure.
> @@ -56,7 +56,7 @@ struct exynos_tmu_data {
> int id;
> struct exynos_tmu_platform_data *pdata;
> void __iomem *base;
> -   void __iomem *base_common;
> +   void __iomem *base_second;
> int irq;
> enum soc_type soc;
> struct work_struct irq_work;
> @@ -297,7 +297,7 @@ skip_calib_data:
> }
> /*Clear the PMIN in the common TMU register*/
> if (reg->tmu_pmin && !data->id)
> -   writel(0, data->base_common + reg->tmu_pmin);
> +   writel(0, data->base_second + reg->tmu_pmin);
>  out:
> clk_disable(data->clk);
> mutex_unlock(&data->lock);
> @@ -454,7 +454,7 @@ static void exynos_tmu_work(struct work_struct *work)
>
> /* Find which sensor generated this interrupt */
> if (reg->tmu_irqstatus) {
> -   val_type = readl(data->base_common + reg->tmu_irqstatus);
> +   val_type = readl(data->base_second + reg->tmu_irqstatus);
> if (!((val_type >> data->id) & 0x1))
> goto out;
> }
> @@ -579,7 +579,7 @@ static int exynos_map_dt_data(struct platform_device 
> *pdev)
>  * Check if the TMU shares some registers and then try to map the
>  * memory of common registers.
>  */
> -   if (!TMU_SUPPORTS(pdata, SHARED_MEMORY))
> +   if (!TMU_SUPPORTS(pdata, ADDRESS_MULTIPLE))
> return 0;
>
> if (of_address_to_resource(pdev->dev.of_node, 1, &res)) {
> @@ -587,9 +587,9 @@ static int exynos_map_dt_data(struct platform_device 
> *pdev)
> return -ENODEV;
> }
>
> -   data->base_common = devm_ioremap(&pdev->dev, res.start,
> +   data->base_second = devm_ioremap(&pdev->dev, res.start,
> resource_size(&res));
> -   if (!data->base_common) {
> +   if (!data->base_second) {
> dev_err(&pdev->dev, "Failed to ioremap memory\n");
> return -ENOMEM;
> }
> diff --git a/drivers/thermal/samsung/exynos_tmu.h 
> b/drivers/thermal/samsung/exynos_tmu.h
> index 980859a..0d6b32f 100644
> --- a/drivers/thermal/samsung/exynos_tmu.h
> +++ b/drivers/thermal/samsung/exynos_tmu.h
> @@ -60,7 +60,7 @@ enum soc_type {
>   * state(active/idle) can be checked.
>   * TMU_SUPPORT_EMUL_TIME - This features allows to set next temp emulation
>   * sample time.
> - * TMU

Re: [PATCH 3/4 v9] thermal: samsung: Add TMU support for Exynos5420 SoCs

2013-11-17 Thread Naveen Krishna Ch
Hello All,

On 12 November 2013 12:07, Naveen Krishna Chatradhi
 wrote:
> Exynos5420 has 5 TMU channels, the TRIMINFO register is
> misplaced for TMU channels 2, 3 and 4
> TRIMINFO at 0x1006c000 contains data for TMU channel 3
> TRIMINFO at 0x100a contains data for TMU channel 4
> TRIMINFO at 0x10068000 contains data for TMU channel 2
>
> This patch
> 1 Adds the neccessary register changes and arch information
>to support Exynos5420 SoCs.
> 2. Handles the gate clock for misplaced TRIMINFO register
> 3. Updates the Documentation at
>Documentation/devicetree/bindings/thermal/exynos-thermal.txt
>
> Signed-off-by: Naveen Krishna Chatradhi 
> Signed-off-by: Andrew Bresticker 
> ---
> Changes since v8:
> 1. rewrote the Documentation for device tree bindings
> 2. Merged the https://lkml.org/lkml/2013/11/7/262 (as this is a fix)
> 3. introduces "samsung,exynos5420-tmu-triminfo" and
>"samsung,exynos5420-tmu-triminfo-clk" to handle the TMU channels on
>Exynos5420 more appropriately
>
>  .../devicetree/bindings/thermal/exynos-thermal.txt |   45 +
>  drivers/thermal/samsung/exynos_tmu.c   |   58 ++-
>  drivers/thermal/samsung/exynos_tmu.h   |2 +
>  drivers/thermal/samsung/exynos_tmu_data.c  |  106 
> 
>  drivers/thermal/samsung/exynos_tmu_data.h  |8 ++
>  5 files changed, 215 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/thermal/exynos-thermal.txt 
> b/Documentation/devicetree/bindings/thermal/exynos-thermal.txt
> index 116cca0..5055b31 100644
> --- a/Documentation/devicetree/bindings/thermal/exynos-thermal.txt
> +++ b/Documentation/devicetree/bindings/thermal/exynos-thermal.txt
> @@ -6,6 +6,11 @@
>"samsung,exynos4412-tmu"
>"samsung,exynos4210-tmu"
>"samsung,exynos5250-tmu"
> +  "samsung,exynos5420-tmu" for TMU channel 0, 1 on Exynos5420
> +  "samsung,exynos5420-tmu-triminfo" for TMU channel 2 Exynos5420
> +   (Must pass triminfo base)
> +  "samsung,exynos5420-tmu-triminfo-clk" for TMU channel 3 and 4
> +   Exynos5420 (Must pass triminfo base and triminfo 
> clock)
>"samsung,exynos5440-tmu"
>  - interrupt-parent : The phandle for the interrupt controller
>  - reg : Address range of the thermal registers. For soc's which has multiple
> @@ -13,6 +18,18 @@
> interrupt related then 2 set of register has to supplied. First set
> belongs to each instance of TMU and second set belongs to second set
> of common TMU registers.
> +
> +  NOTE: On Exynos5420, the TRIMINFO register is misplaced for TMU
> +   channels 2, 3 and 4
> +   Use "samsung,exynos5420-tmu-triminfo" in cases, there is a misplaced
> +   register but no need of another clock to access that base.
> +   Use "samsung,exynos5420-tmu-triminfo-clk" in cases where there is a 
> misplaced
> +   register and we need another clock to access that base.
> +
> +   TRIMINFO at 0x1006c000 contains data for TMU channel 3
> +   TRIMINFO at 0x100a contains data for TMU channel 4
> +   TRIMINFO at 0x10068000 contains data for TMU channel 2
> +
>  - interrupts : Should contain interrupt for thermal system
>  - clocks : The main clock for TMU device
>  - clock-names : Thermal system clock name
> @@ -43,6 +60,34 @@ Example 2):
> clock-names = "tmu_apbif";
> };
>
> +Example 3): (In case of Exynos5420 "with misplaced TRIMINFO register")
> +   /* tmu for CPU2 */
> +   tmu@10068000 {
> +   compatible = "samsung,exynos5420-tmu-triminfo";
> +   reg = <0x10068000 0x100>, <0x1006c000 0x4>;
> +   interrupts = <0 184 0>;
> +   clocks = <&clock 318>;
> +   clock-names = "tmu_apbif";
> +   };
> +
> +   /* tmu for CPU3 */
> +   tmu@1006c000 {
> +   compatible = "samsung,exynos5420-tmu-triminfo-clk";
> +   reg = <0x1006c000 0x100>, <0x100a 0x4>;
> +   interrupts = <0 185 0>;
> +   clocks = <&clock 318>;
> +   clock-names = "tmu_apbif", "tmu_triminfo_apbif";
> +   };
> +
> +   /* tmu for GPU */
> +   tmu@100a {
> +   compatible = "samsung,exynos5420-tmu-triminfo-clk";
> +   reg = <0x100a 0x100>, <0x10068000 0x4>;
> +   interrupts = <0 215 0>;
> +   clocks = <&clock 318>;
> +   clock-names = "tmu_apbif", "tmu_triminfo_apbif";
> +   };
> +
>  Note: For multi-instance tmu each instance should have an alias correctly
>  numbered in "aliases" node.
>
> diff --git a/drivers/thermal/samsung/exynos_tmu.c 
> b/drivers/thermal/samsung/exynos_tmu.c
> index bbd0fc3..826647c 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -47,6 +47,7 @@
>   * @irq_work: pointer to the irq work 

Re: [PATCH 4/4 v3] ARM: dts: Exynos5420: Add device nodes for TMU blocks

2013-11-17 Thread Naveen Krishna Ch
Hello All,

On 12 November 2013 12:07, Naveen Krishna Chatradhi
 wrote:
> Exynos5420 SoC has per core thermal management unit.
> 5 TMU channels 4 for CPUs and 5th for GPU.
>
> This patch adds the device tree nodes to the DT device list.
>
> Nodes carry the misplaced second base address and the second
> clock to access the misplaced base address.
>
> Signed-off-by: Leela Krishna Amudala 
> Signed-off-by: Naveen Krishna Chatradhi 
> Signed-off-by: Andrew Bresticker 
> ---
> Changes since v2:
> 3. uses the new compatible strings introduced along with adding
>support for Exynso5420.
>
> Changes since v1:
> 1. Nodes carry the misplaced second base address and the second
>clock to access the misplaced base address.
> 2. Correct the clock number for the TMU4
>
>  arch/arm/boot/dts/exynos5420.dtsi |   48 
> +
>  1 file changed, 48 insertions(+)
>
> diff --git a/arch/arm/boot/dts/exynos5420.dtsi 
> b/arch/arm/boot/dts/exynos5420.dtsi
> index 6ffefd1..d736b40 100644
> --- a/arch/arm/boot/dts/exynos5420.dtsi
> +++ b/arch/arm/boot/dts/exynos5420.dtsi
> @@ -369,4 +369,52 @@
> clock-names = "gscl";
> samsung,power-domain = <&gsc_pd>;
> };
> +
> +   /* tmu for CPU0 */
> +   tmu@1006 {
> +   compatible = "samsung,exynos5420-tmu";
> +   reg = <0x1006 0x100>;
> +   interrupts = <0 65 0>;
> +   clocks = <&clock 318>;
> +   clock-names = "tmu_apbif";
> +   };
> +
> +   /* tmu for CPU1 */
> +   tmu@10064000 {
> +   compatible = "samsung,exynos5420-tmu";
> +   reg = <0x10064000 0x100>;
> +   interrupts = <0 183 0>;
> +   clocks = <&clock 318>;
> +   clock-names = "tmu_apbif";
> +   };
> +
> +   /* tmu for CPU2 */
> +   tmu@10068000 {
> +   compatible = "samsung,exynos5420-tmu-triminfo";
> +   /* 2nd reg is for the misplaced TRIMINFO register */
> +   reg = <0x10068000 0x100>, <0x1006c000 0x4>;
> +   interrupts = <0 184 0>;
> +   clocks = <&clock 318>;
> +   clock-names = "tmu_apbif";
> +   };
> +
> +   /* tmu for CPU3 */
> +   tmu@1006c000 {
> +   compatible = "samsung,exynos5420-tmu-triminfo-clk";
> +   /* 2nd reg is for the misplaced TRIMINFO register */
> +   reg = <0x1006c000 0x100>, <0x100a 0x4>;
> +   interrupts = <0 185 0>;
> +   clocks = <&clock 318>, <&clock 319>;
> +   clock-names = "tmu_apbif", "tmu_apbif_triminfo";
> +   };
> +
> +   /* tmu for GPU */
> +   tmu@100a {
> +   compatible = "samsung,exynos5420-tmu-triminfo-clk";
> +   /* 2nd reg is for the misplaced TRIMINFO register */
> +   reg = <0x100a 0x100>, <0x10068000 0x4>;
> +   interrupts = <0 215 0>;
> +   clocks = <&clock 319>, <&clock 318>;
> +   clock-names = "tmu_apbif", "tmu_apbif_triminfo";
> +   };
>  };
> --
> 1.7.10.4
Any comments on this patch
>



-- 
Shine bright,
(: Nav :)
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ARM: move firmware_ops to drivers/firmware

2013-11-17 Thread Alex Courbot

On 11/18/2013 12:59 AM, Catalin Marinas wrote:

On 17 November 2013 08:49, Alexandre Courbot  wrote:

The ARM tree includes a firmware_ops interface that is designed to
implement support for simple, TrustZone-based firmwares but could
also cover other use-cases. It has been suggested that this
interface might be useful to other architectures (e.g. arm64) and
that it should be moved out of arch/arm.


NAK. I'm for code sharing with arm via common locations but this API
goes against the ARMv8 firmware standardisation efforts like PSCI,
encouraging each platform to define there own non-standard interface.


I have to say, I pretty much agree with your NAK.

The reason for this patch is that the suggestion to move firmware_ops 
out of arch/arm is the last (I hope) thing that prevents my Trusted 
Foundation support series from being merged. Now if we can all agree:


* that ARMv8 will only use PSCI
* that there is no use-case of this interface outside of arch/arm as of 
today (and none foreseen in the near future)

* that the firmware_ops interface is quite ARMv7-specific anyway,
* that should a need to move it (for whatever reason) occur later, it 
will be easy to do (as this patch hopefully demonstrates).


... then this has indeed no reason to be. And maybe I can finally get 
Russell's blessing on my series.





--- /dev/null
+++ b/include/linux/platform_firmware.h
@@ -0,0 +1,69 @@
+/*
+ * Copyright (C) 2012 Samsung Electronics.
+ * Kyungmin Park 
+ * Tomasz Figa 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _PLATFORM_FIRMWARE_H
+#define _PLATFORM_FIRMWARE_H
+
+#include 
+
+/*
+ * struct platform_firmware_ops
+ *
+ * A structure to specify available firmware operations.
+ *
+ * A filled up structure can be registered with
+ * register_platform_firmware_ops().
+ */
+struct platform_firmware_ops {
+   /*
+* Enters CPU idle mode
+*/
+   int (*do_idle)(void);


Covered by PSCI already.


+   /*
+* Sets boot address of specified physical CPU
+*/
+   int (*set_cpu_boot_addr)(int cpu, unsigned long boot_addr);


Covered either by PSCI or spin-table release method (PSCI if firmware
call is required).


+   /*
+* Boots specified physical CPU
+*/
+   int (*cpu_boot)(int cpu);


PSCI.


+   /*
+* Initializes L2 cache
+*/
+   int (*l2x0_init)(void);


No L2x0 (L210, L220, PL310) cache on ARMv8. And here I strongly
recommend the hardware people to make proper external caches which can
be flushed by standard CPU instructions, not MMIO. Any such caches
must be enabled by firmware before Linux starts.

The above firmware API is 32-bit ARM only. Form 64-bit ARM, you have
the choice of PSCI so far but as I said in a long thread to Nico, I'm
open to other standard interfaces if there are good reasons PSCI
cannot be used. Note the _standard_ part, I don't want every SoC with
their own firmware API for standard things like secondary CPU
booting/hotplug/idle.



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


Re: [PATCH] ARM: move firmware_ops to drivers/firmware

2013-11-17 Thread Catalin Marinas
On 17 November 2013 08:49, Alexandre Courbot  wrote:
> The ARM tree includes a firmware_ops interface that is designed to
> implement support for simple, TrustZone-based firmwares but could
> also cover other use-cases. It has been suggested that this
> interface might be useful to other architectures (e.g. arm64) and
> that it should be moved out of arch/arm.

NAK. I'm for code sharing with arm via common locations but this API
goes against the ARMv8 firmware standardisation efforts like PSCI,
encouraging each platform to define there own non-standard interface.

> --- /dev/null
> +++ b/include/linux/platform_firmware.h
> @@ -0,0 +1,69 @@
> +/*
> + * Copyright (C) 2012 Samsung Electronics.
> + * Kyungmin Park 
> + * Tomasz Figa 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef _PLATFORM_FIRMWARE_H
> +#define _PLATFORM_FIRMWARE_H
> +
> +#include 
> +
> +/*
> + * struct platform_firmware_ops
> + *
> + * A structure to specify available firmware operations.
> + *
> + * A filled up structure can be registered with
> + * register_platform_firmware_ops().
> + */
> +struct platform_firmware_ops {
> +   /*
> +* Enters CPU idle mode
> +*/
> +   int (*do_idle)(void);

Covered by PSCI already.

> +   /*
> +* Sets boot address of specified physical CPU
> +*/
> +   int (*set_cpu_boot_addr)(int cpu, unsigned long boot_addr);

Covered either by PSCI or spin-table release method (PSCI if firmware
call is required).

> +   /*
> +* Boots specified physical CPU
> +*/
> +   int (*cpu_boot)(int cpu);

PSCI.

> +   /*
> +* Initializes L2 cache
> +*/
> +   int (*l2x0_init)(void);

No L2x0 (L210, L220, PL310) cache on ARMv8. And here I strongly
recommend the hardware people to make proper external caches which can
be flushed by standard CPU instructions, not MMIO. Any such caches
must be enabled by firmware before Linux starts.

The above firmware API is 32-bit ARM only. Form 64-bit ARM, you have
the choice of PSCI so far but as I said in a long thread to Nico, I'm
open to other standard interfaces if there are good reasons PSCI
cannot be used. Note the _standard_ part, I don't want every SoC with
their own firmware API for standard things like secondary CPU
booting/hotplug/idle.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] ARM: move firmware_ops to drivers/firmware

2013-11-17 Thread Alexandre Courbot
The ARM tree includes a firmware_ops interface that is designed to
implement support for simple, TrustZone-based firmwares but could
also cover other use-cases. It has been suggested that this
interface might be useful to other architectures (e.g. arm64) and
that it should be moved out of arch/arm.

This patch takes care of this by performing the following:
* Move the ARM firmware interface into drivers/firmware/ and rename
  it to platform_firmware,
* Update its documentation accordingly,
* Update the only user of the API to date (Exynos secure firmware
  support) to use the new API.

The ARM architecture's Kconfig now needs to include the Kconfig of
drivers/firmware, which will result in an empty menu for most platforms
that don't make use of any firmware. To avoid this, a new invisible
ARCH_SUPPORTS_FIRMWARE configuration variable is also defined for ARM,
that should explicitly be set by platforms that support firmware so that
the firmware menu is included.

Signed-off-by: Alexandre Courbot 
---
 Documentation/arm/firmware.txt   | 88 ---
 Documentation/firmware/platform_firmware.txt | 89 
 arch/arm/Kconfig |  9 +++
 arch/arm/common/Makefile |  2 -
 arch/arm/common/firmware.c   | 18 --
 arch/arm/include/asm/firmware.h  | 66 -
 arch/arm/mach-exynos/Makefile|  2 +-
 arch/arm/mach-exynos/firmware.c  |  7 +--
 arch/arm/mach-exynos/platsmp.c   | 10 ++--
 drivers/firmware/Kconfig |  8 +++
 drivers/firmware/Makefile|  1 +
 drivers/firmware/platform_firmware.c | 17 ++
 include/linux/platform_firmware.h| 69 +
 13 files changed, 203 insertions(+), 183 deletions(-)
 delete mode 100644 Documentation/arm/firmware.txt
 create mode 100644 Documentation/firmware/platform_firmware.txt
 delete mode 100644 arch/arm/common/firmware.c
 delete mode 100644 arch/arm/include/asm/firmware.h
 create mode 100644 drivers/firmware/platform_firmware.c
 create mode 100644 include/linux/platform_firmware.h

diff --git a/Documentation/arm/firmware.txt b/Documentation/arm/firmware.txt
deleted file mode 100644
index c2e468f..000
--- a/Documentation/arm/firmware.txt
+++ /dev/null
@@ -1,88 +0,0 @@
-Interface for registering and calling firmware-specific operations for ARM.
-
-Written by Tomasz Figa 
-
-Some boards are running with secure firmware running in TrustZone secure
-world, which changes the way some things have to be initialized. This makes
-a need to provide an interface for such platforms to specify available firmware
-operations and call them when needed.
-
-Firmware operations can be specified using struct firmware_ops
-
-   struct firmware_ops {
-   /*
-   * Enters CPU idle mode
-   */
-   int (*do_idle)(void);
-   /*
-   * Sets boot address of specified physical CPU
-   */
-   int (*set_cpu_boot_addr)(int cpu, unsigned long boot_addr);
-   /*
-   * Boots specified physical CPU
-   */
-   int (*cpu_boot)(int cpu);
-   /*
-   * Initializes L2 cache
-   */
-   int (*l2x0_init)(void);
-   };
-
-and then registered with register_firmware_ops function
-
-   void register_firmware_ops(const struct firmware_ops *ops)
-
-the ops pointer must be non-NULL.
-
-There is a default, empty set of operations provided, so there is no need to
-set anything if platform does not require firmware operations.
-
-To call a firmware operation, a helper macro is provided
-
-   #define call_firmware_op(op, ...)   \
-   ((firmware_ops->op) ? firmware_ops->op(__VA_ARGS__) : (-ENOSYS))
-
-the macro checks if the operation is provided and calls it or otherwise returns
--ENOSYS to signal that given operation is not available (for example, to allow
-fallback to legacy operation).
-
-Example of registering firmware operations:
-
-   /* board file */
-
-   static int platformX_do_idle(void)
-   {
-   /* tell platformX firmware to enter idle */
-   return 0;
-   }
-
-   static int platformX_cpu_boot(int i)
-   {
-   /* tell platformX firmware to boot CPU i */
-   return 0;
-   }
-
-   static const struct firmware_ops platformX_firmware_ops = {
-   .do_idle= exynos_do_idle,
-   .cpu_boot   = exynos_cpu_boot,
-   /* other operations not available on platformX */
-   };
-
-   /* init_early callback of machine descriptor */
-   static void __init board_init_early(void)
-   {
-   register_firmware_ops(&platformX_firmware_ops);
-   }
-
-Example of using a firmware operation:
-