Re: [PATCH] phy: add axis artpec8 pcie phy driver

[Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>]

On Fri, Jan 28, 2022 at 03:01:42PM +0900, 이왕석 wrote:
> add axis artpec8 pcie phy driver
> 
> Signed-off-by: wangseok.lee <wangseok.lee@samsung.com>
> ---
>  drivers/phy/artpec/Kconfig            |   7 +
>  drivers/phy/artpec/Makefile           |   2 +
>  drivers/phy/artpec/phy-artpec8-pcie.c | 880 ++++++++++++++++++++++++++++++++++
>  3 files changed, 889 insertions(+)
>  create mode 100644 drivers/phy/artpec/Kconfig
>  create mode 100644 drivers/phy/artpec/Makefile
>  create mode 100644 drivers/phy/artpec/phy-artpec8-pcie.c
> 
> diff --git a/drivers/phy/artpec/Kconfig b/drivers/phy/artpec/Kconfig
> new file mode 100644
> index 0000000..68c5ecb
> --- /dev/null
> +++ b/drivers/phy/artpec/Kconfig
> @@ -0,0 +1,7 @@
> +config PHY_ARTPEC8_PCIE
> +	bool "ARTPEC-8 PCIe PHY driver"
> +	depends on OF && (ARCH_ARTPEC8 || COMPILE_TEST)

What is ARCH_ARTPEC8?

> +	select GENERIC_PHY
> +	help
> +	  Enable PCIe PHY support for ARTPEC-8.
> +	  This driver provides PHY interface for ARTPEC-8 PCIe controller.
> diff --git a/drivers/phy/artpec/Makefile b/drivers/phy/artpec/Makefile
> new file mode 100644
> index 0000000..45d853c
> --- /dev/null
> +++ b/drivers/phy/artpec/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_PHY_ARTPEC8_PCIE)		+= phy-artpec8-pcie.o
> diff --git a/drivers/phy/artpec/phy-artpec8-pcie.c b/drivers/phy/artpec/phy-artpec8-pcie.c
> new file mode 100644
> index 0000000..1d1f37f
> --- /dev/null
> +++ b/drivers/phy/artpec/phy-artpec8-pcie.c
> @@ -0,0 +1,880 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * PHY provider for ARTPEC-8 PCIe controller
> + *
> + * Copyright (C) 2019 Samsung Electronics Co., Ltd.
> + *		http://www.samsung.com
> + *
> + * Author: Jaeho Cho <jaeho79.cho@samsung.com>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/init.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/phy/phy.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/samsung-sysmgr.h>

This does not exist. I don't think you compiled it. If that's the case,
such code should not be posted to the mailing list...

One more comment later but in general there is no point in reviewing
unbuildable code.

> +#include <linux/debugfs.h>
> +
> +/* ARTPEC-8 PCIe PHY registers */
> +/* CMN registers */

(...)

> +
> +static const struct phy_ops artpec8_phy_ops = {
> +	.init		= artpec8_pcie_phy_init,
> +	.exit		= artpec8_pcie_phy_exit,
> +	.reset		= artpec8_pcie_phy_reset,
> +	.power_on	= artpec8_pcie_phy_power_on,
> +	.power_off	= artpec8_pcie_phy_power_off,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static const struct artpec8_pcie_phy_data artpec8_pcie_phy_data = {
> +	.ops		= &artpec8_phy_ops,
> +};
> +
> +static const struct of_device_id artpec8_pcie_phy_match[] = {
> +	{
> +		.compatible = "samsung,artpec8-pcie-phy",

Where is it documented? Did you run checkpatch?


Best regards,
Krzysztof
> +
> +	if (artpec8_phy->soc_pll_clk)
> +		clk_disable_unprepare(artpec8_phy->soc_pll_clk);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver artpec8_pcie_phy_driver = {
> +	.probe	= artpec8_pcie_phy_probe,
> +	.remove	= __exit_p(artpec8_pcie_phy_remove),
> +	.driver = {
> +		.of_match_table	= artpec8_pcie_phy_match,
> +		.name		= "artpec8_pcie_phy",
> +	}
> +};
> +
> +module_platform_driver(artpec8_pcie_phy_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Jaeho Cho <jaeho79.cho@samsung.com>");
> -- 
> 2.9.5
> 
> -- 
> linux-phy mailing list
> linux-phy@lists.infradead.org
> https://lists.infradead.org/mailman/listinfo/linux-phy

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy