lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Tue, 3 Dec 2013 20:16:26 +0530
From:	Abhilash Kesavan <kesavan.abhilash@...il.com>
To:	Sachin Kamat <sachin.kamat@...aro.org>
Cc:	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	linux-samsung-soc <linux-samsung-soc@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Kukjin Kim <kgene.kim@...sung.com>,
	Tomasz Figa <tomasz.figa@...il.com>,
	Yadwinder Singh Brar <yadi.brar01@...il.com>,
	"myungjoo.ham" <myungjoo.ham@...sung.com>,
	Yadwinder Singh Brar <yadi.brar@...sung.com>,
	Andrew Bresticker <abrestic@...gle.com>,
	Doug Anderson <dianders@...gle.com>
Subject: Re: [RFC v2 1/4] power: asv: Add common ASV support for Samsung SoCs

Hi,

CC'ing Doug and Andrew who have also worked on ASV.

[...]

> diff --git a/drivers/power/asv/Kconfig b/drivers/power/asv/Kconfig
> new file mode 100644
> index 000000000000..761119d9f7f8
> --- /dev/null
> +++ b/drivers/power/asv/Kconfig
> @@ -0,0 +1,10 @@
> +menuconfig POWER_ASV
> +       bool "Adaptive Supply Voltage (ASV) support"
Select POWER_SUPPLY here ?
> +       help
> +         ASV is a technique used on Samsung SoCs which provides the
> +         recommended supply voltage for some specific parts(like CPU, MIF, etc)
> +         that support DVFS. For a given operating frequency, the voltage is
> +         recommended based on SoCs ASV group. ASV group info is provided in the
> +         chip id info which depends on the chip manufacturing process.
> +

[...]
> +
> +       if (asv_info->ops->init_asv)
> +               ret = asv_info->ops->init_asv(asv_info);
> +               if (ret) {
> +                       pr_err("asv_init failed for %s : %d\n",
> +                               asv_info->name, ret);
> +                       goto err;
> +               }
> +
> +       /* In case of parsing table from DT, we may need to add flag to identify
> +       DT supporting members and call init_asv_table from asv_init_opp_table(
> +       after getting dev_node from dev,if required), instead of calling here.
> +       */
Please fix Multi-line comment here and through the rest of the patches as well.

[...]
> + * @nr_dvfs_level: Number of dvfs levels supported by member.
> + * @dvfs_table: Table containing supported ASV freqs and corresponding volts.
> + * @asv_grp: ASV group of member.
> + * @flags: ASV flags
What are the ASV flags you had in mind ?
> + */
> +struct asv_info {
> +       const char              *name;
> +       enum asv_type_id        type;
> +       struct asv_ops          *ops;
> +       unsigned int            nr_dvfs_level;
> +       struct asv_freq_table   *dvfs_table;
> +       unsigned int            asv_grp;
> +       unsigned int            flags;
> +};

[...]
> +
> +#ifdef CONFIG_POWER_ASV
> +/* asv_get_volt - get the ASV for target_freq for particular target_type.
> + *     returns 0 if target_freq is not supported
Could you add a comment for asv_init_opp_table as well.
> + */
> +extern unsigned int asv_get_volt(enum asv_type_id target_type,
> +                                       unsigned int target_freq);
> +extern int asv_init_opp_table(struct device *dev,
> +                                       enum asv_type_id target_type);
> +#else
> +static inline unsigned int asv_get_volt(enum asv_type_id target_type,
> +                               unsigned int target_freq) { return 0; }
> +static int asv_init_opp_table(struct device *dev, enum asv_type_id target_type)
> +       { return 0; }
> +
> +#endif /* CONFIG_POWER_EXYNOS_AVS */
> +#endif /* __ASV_H */

Regards,
Abhilash
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists