Message ID | 20240506170110.2874724-5-sean.anderson@linux.dev |
---|---|
State | Superseded |
Headers | show |
Series | phy: zynqmp: Fixes and debugfs support | expand |
> -----Original Message----- > From: Sean Anderson <sean.anderson@linux.dev> > Sent: Monday, May 6, 2024 10:31 PM > To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>; linux- > phy@lists.infradead.org > Cc: Vinod Koul <vkoul@kernel.org>; linux-arm-kernel@lists.infradead.org; > linux-kernel@vger.kernel.org; Michal Simek <michal.simek@amd.com>; > Kishon Vijay Abraham I <kishon@kernel.org>; Sean Anderson > <sean.anderson@linux.dev> > Subject: [PATCH v2 4/4] phy: zynqmp: Add debugfs support > > Add support for printing some basic status information to debugfs. This > is helpful when debugging phy consumers to make sure they are configuring > the phy appropriately. > > Signed-off-by: Sean Anderson <sean.anderson@linux.dev> > --- > > Changes in v2: > - Use debugfs_create_devm_seqfile > > drivers/phy/xilinx/phy-zynqmp.c | 40 > +++++++++++++++++++++++++++++++++ > 1 file changed, 40 insertions(+) > > diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy- > zynqmp.c > index b86fe2a249a8..2520c75a4a77 100644 > --- a/drivers/phy/xilinx/phy-zynqmp.c > +++ b/drivers/phy/xilinx/phy-zynqmp.c > @@ -13,6 +13,7 @@ > */ > > #include <linux/clk.h> > +#include <linux/debugfs.h> > #include <linux/delay.h> > #include <linux/io.h> > #include <linux/kernel.h> > @@ -123,6 +124,15 @@ > #define ICM_PROTOCOL_DP 0x4 > #define ICM_PROTOCOL_SGMII 0x5 > > +static const char *const xpsgtr_icm_str[] = { > + [ICM_PROTOCOL_PD] = "powered down", Protocol saying powered down seems confusing. Should we say None or None(power down)? > + [ICM_PROTOCOL_PCIE] = "PCIe", > + [ICM_PROTOCOL_SATA] = "SATA", > + [ICM_PROTOCOL_USB] = "USB", > + [ICM_PROTOCOL_DP] = "DisplayPort", > + [ICM_PROTOCOL_SGMII] = "SGMII", > +}; > + > /* Test Mode common reset control parameters */ > #define TM_CMN_RST 0x10018 > #define TM_CMN_RST_EN 0x1 > @@ -788,6 +798,34 @@ static struct phy *xpsgtr_xlate(struct device *dev, > return ERR_PTR(-EINVAL); > } > > +/* > + * DebugFS > + */ > + > +static int xpsgtr_status_read(struct seq_file *seq, void *data) > +{ > + struct device *dev = seq->private; > + struct xpsgtr_phy *gtr_phy = dev_get_drvdata(dev); > + struct clk *clk; > + u32 pll_status; > + > + mutex_lock(>r_phy->phy->mutex); Do we see any need for this lock? This function simply read hw register /phy members and decodes it. > + pll_status = xpsgtr_read_phy(gtr_phy, L0_PLL_STATUS_READ_1); > + clk = gtr_phy->dev->clk[gtr_phy->refclk]; > + > + seq_printf(seq, "Lane: %u\n", gtr_phy->lane); > + seq_printf(seq, "Protocol: %s\n", > + xpsgtr_icm_str[gtr_phy->protocol]); > + seq_printf(seq, "Instance: %u\n", gtr_phy->instance); > + seq_printf(seq, "Reference clock: %u (%pC)\n", gtr_phy->refclk, clk); > + seq_printf(seq, "Reference rate: %lu\n", clk_get_rate(clk)); > + seq_printf(seq, "PLL locked: %s\n", > + pll_status & PLL_STATUS_LOCKED ? "yes" : "no"); > + > + mutex_unlock(>r_phy->phy->mutex); > + return 0; > +} > + > /* > * Power Management > */ > @@ -937,6 +975,8 @@ static int xpsgtr_probe(struct platform_device *pdev) > > gtr_phy->phy = phy; > phy_set_drvdata(phy, gtr_phy); > + debugfs_create_devm_seqfile(&phy->dev, "status", phy- > >debugfs, > + xpsgtr_status_read); > } > > /* Register the PHY provider. */ > -- > 2.35.1.1320.gc452695387.dirty
On 6/13/24 05:20, Pandey, Radhey Shyam wrote: >> -----Original Message----- >> From: Sean Anderson <sean.anderson@linux.dev> >> Sent: Monday, May 6, 2024 10:31 PM >> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>; linux- >> phy@lists.infradead.org >> Cc: Vinod Koul <vkoul@kernel.org>; linux-arm-kernel@lists.infradead.org; >> linux-kernel@vger.kernel.org; Michal Simek <michal.simek@amd.com>; >> Kishon Vijay Abraham I <kishon@kernel.org>; Sean Anderson >> <sean.anderson@linux.dev> >> Subject: [PATCH v2 4/4] phy: zynqmp: Add debugfs support >> >> Add support for printing some basic status information to debugfs. This >> is helpful when debugging phy consumers to make sure they are configuring >> the phy appropriately. >> >> Signed-off-by: Sean Anderson <sean.anderson@linux.dev> >> --- >> >> Changes in v2: >> - Use debugfs_create_devm_seqfile >> >> drivers/phy/xilinx/phy-zynqmp.c | 40 >> +++++++++++++++++++++++++++++++++ >> 1 file changed, 40 insertions(+) >> >> diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy- >> zynqmp.c >> index b86fe2a249a8..2520c75a4a77 100644 >> --- a/drivers/phy/xilinx/phy-zynqmp.c >> +++ b/drivers/phy/xilinx/phy-zynqmp.c >> @@ -13,6 +13,7 @@ >> */ >> >> #include <linux/clk.h> >> +#include <linux/debugfs.h> >> #include <linux/delay.h> >> #include <linux/io.h> >> #include <linux/kernel.h> >> @@ -123,6 +124,15 @@ >> #define ICM_PROTOCOL_DP 0x4 >> #define ICM_PROTOCOL_SGMII 0x5 >> >> +static const char *const xpsgtr_icm_str[] = { >> + [ICM_PROTOCOL_PD] = "powered down", > > Protocol saying powered down seems confusing. > Should we say None or None(power down)? Either works I guess. I'm just matching the define. >> + [ICM_PROTOCOL_PCIE] = "PCIe", >> + [ICM_PROTOCOL_SATA] = "SATA", >> + [ICM_PROTOCOL_USB] = "USB", >> + [ICM_PROTOCOL_DP] = "DisplayPort", >> + [ICM_PROTOCOL_SGMII] = "SGMII", >> +}; >> + >> /* Test Mode common reset control parameters */ >> #define TM_CMN_RST 0x10018 >> #define TM_CMN_RST_EN 0x1 >> @@ -788,6 +798,34 @@ static struct phy *xpsgtr_xlate(struct device *dev, >> return ERR_PTR(-EINVAL); >> } >> >> +/* >> + * DebugFS >> + */ >> + >> +static int xpsgtr_status_read(struct seq_file *seq, void *data) >> +{ >> + struct device *dev = seq->private; >> + struct xpsgtr_phy *gtr_phy = dev_get_drvdata(dev); >> + struct clk *clk; >> + u32 pll_status; >> + >> + mutex_lock(>r_phy->phy->mutex); > > Do we see any need for this lock? This function simply read hw register > /phy members and decodes it. It's to protect against modifications to gtr_phy->refclk and ->instance. These are accessed under lock elsewhere; this is not a fast path and I don't want to have to reason about the semantics when using a mutex is definitely correct. --Sean >> + pll_status = xpsgtr_read_phy(gtr_phy, L0_PLL_STATUS_READ_1); >> + clk = gtr_phy->dev->clk[gtr_phy->refclk]; >> + >> + seq_printf(seq, "Lane: %u\n", gtr_phy->lane); >> + seq_printf(seq, "Protocol: %s\n", >> + xpsgtr_icm_str[gtr_phy->protocol]); >> + seq_printf(seq, "Instance: %u\n", gtr_phy->instance); >> + seq_printf(seq, "Reference clock: %u (%pC)\n", gtr_phy->refclk, clk); >> + seq_printf(seq, "Reference rate: %lu\n", clk_get_rate(clk)); >> + seq_printf(seq, "PLL locked: %s\n", >> + pll_status & PLL_STATUS_LOCKED ? "yes" : "no"); >> + >> + mutex_unlock(>r_phy->phy->mutex); >> + return 0; >> +} >> + >> /* >> * Power Management >> */ >> @@ -937,6 +975,8 @@ static int xpsgtr_probe(struct platform_device *pdev) >> >> gtr_phy->phy = phy; >> phy_set_drvdata(phy, gtr_phy); >> + debugfs_create_devm_seqfile(&phy->dev, "status", phy- >> >debugfs, >> + xpsgtr_status_read); >> } >> >> /* Register the PHY provider. */ >> -- >> 2.35.1.1320.gc452695387.dirty >
> -----Original Message----- > From: Sean Anderson <sean.anderson@linux.dev> > Sent: Thursday, June 13, 2024 8:32 PM > To: Pandey, Radhey Shyam <radhey.shyam.pandey@amd.com>; Laurent > Pinchart <laurent.pinchart@ideasonboard.com>; linux- > phy@lists.infradead.org > Cc: Vinod Koul <vkoul@kernel.org>; linux-arm-kernel@lists.infradead.org; > linux-kernel@vger.kernel.org; Simek, Michal <michal.simek@amd.com>; > Kishon Vijay Abraham I <kishon@kernel.org> > Subject: Re: [PATCH v2 4/4] phy: zynqmp: Add debugfs support > > On 6/13/24 05:20, Pandey, Radhey Shyam wrote: > >> -----Original Message----- > >> From: Sean Anderson <sean.anderson@linux.dev> > >> Sent: Monday, May 6, 2024 10:31 PM > >> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>; linux- > >> phy@lists.infradead.org > >> Cc: Vinod Koul <vkoul@kernel.org>; linux-arm-kernel@lists.infradead.org; > >> linux-kernel@vger.kernel.org; Michal Simek <michal.simek@amd.com>; > >> Kishon Vijay Abraham I <kishon@kernel.org>; Sean Anderson > >> <sean.anderson@linux.dev> > >> Subject: [PATCH v2 4/4] phy: zynqmp: Add debugfs support > >> > >> Add support for printing some basic status information to debugfs. This > >> is helpful when debugging phy consumers to make sure they are > configuring > >> the phy appropriately. > >> > >> Signed-off-by: Sean Anderson <sean.anderson@linux.dev> > >> --- > >> > >> Changes in v2: > >> - Use debugfs_create_devm_seqfile > >> > >> drivers/phy/xilinx/phy-zynqmp.c | 40 > >> +++++++++++++++++++++++++++++++++ > >> 1 file changed, 40 insertions(+) > >> > >> diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy- > >> zynqmp.c > >> index b86fe2a249a8..2520c75a4a77 100644 > >> --- a/drivers/phy/xilinx/phy-zynqmp.c > >> +++ b/drivers/phy/xilinx/phy-zynqmp.c > >> @@ -13,6 +13,7 @@ > >> */ > >> > >> #include <linux/clk.h> > >> +#include <linux/debugfs.h> > >> #include <linux/delay.h> > >> #include <linux/io.h> > >> #include <linux/kernel.h> > >> @@ -123,6 +124,15 @@ > >> #define ICM_PROTOCOL_DP 0x4 > >> #define ICM_PROTOCOL_SGMII 0x5 > >> > >> +static const char *const xpsgtr_icm_str[] = { > >> + [ICM_PROTOCOL_PD] = "powered down", > > > > Protocol saying powered down seems confusing. > > Should we say None or None(power down)? > > Either works I guess. I'm just matching the define. None seems fine - we can respin if there are no objections. > > >> + [ICM_PROTOCOL_PCIE] = "PCIe", > >> + [ICM_PROTOCOL_SATA] = "SATA", > >> + [ICM_PROTOCOL_USB] = "USB", > >> + [ICM_PROTOCOL_DP] = "DisplayPort", > >> + [ICM_PROTOCOL_SGMII] = "SGMII", > >> +}; > >> + > >> /* Test Mode common reset control parameters */ > >> #define TM_CMN_RST 0x10018 > >> #define TM_CMN_RST_EN 0x1 > >> @@ -788,6 +798,34 @@ static struct phy *xpsgtr_xlate(struct device *dev, > >> return ERR_PTR(-EINVAL); > >> } > >> > >> +/* > >> + * DebugFS > >> + */ > >> + > >> +static int xpsgtr_status_read(struct seq_file *seq, void *data) > >> +{ > >> + struct device *dev = seq->private; > >> + struct xpsgtr_phy *gtr_phy = dev_get_drvdata(dev); > >> + struct clk *clk; > >> + u32 pll_status; > >> + > >> + mutex_lock(>r_phy->phy->mutex); > > > > Do we see any need for this lock? This function simply read hw register > > /phy members and decodes it. > > It's to protect against modifications to gtr_phy->refclk and ->instance. Refclock and instance is assigned in xlate which is not protected by any Lock. Also xpsgtr_phy_init () uses a different gtr_mutex. So want to understand this phy->mutex need. > > These are accessed under lock elsewhere; this is not a fast path and I don't > want to have to reason about the semantics when using a mutex is definitely > correct. > > --Sean > > >> + pll_status = xpsgtr_read_phy(gtr_phy, L0_PLL_STATUS_READ_1); > >> + clk = gtr_phy->dev->clk[gtr_phy->refclk]; > >> + > >> + seq_printf(seq, "Lane: %u\n", gtr_phy->lane); > >> + seq_printf(seq, "Protocol: %s\n", > >> + xpsgtr_icm_str[gtr_phy->protocol]); > >> + seq_printf(seq, "Instance: %u\n", gtr_phy->instance); > >> + seq_printf(seq, "Reference clock: %u (%pC)\n", gtr_phy->refclk, clk); > >> + seq_printf(seq, "Reference rate: %lu\n", clk_get_rate(clk)); > >> + seq_printf(seq, "PLL locked: %s\n", > >> + pll_status & PLL_STATUS_LOCKED ? "yes" : "no"); > >> + > >> + mutex_unlock(>r_phy->phy->mutex); > >> + return 0; > >> +} > >> + > >> /* > >> * Power Management > >> */ > >> @@ -937,6 +975,8 @@ static int xpsgtr_probe(struct platform_device > *pdev) > >> > >> gtr_phy->phy = phy; > >> phy_set_drvdata(phy, gtr_phy); > >> + debugfs_create_devm_seqfile(&phy->dev, "status", phy- > >> >debugfs, > >> + xpsgtr_status_read); > >> } > >> > >> /* Register the PHY provider. */ > >> -- > >> 2.35.1.1320.gc452695387.dirty > >
On 6/14/24 01:16, Pandey, Radhey Shyam wrote: >> -----Original Message----- >> From: Sean Anderson <sean.anderson@linux.dev> >> Sent: Thursday, June 13, 2024 8:32 PM >> To: Pandey, Radhey Shyam <radhey.shyam.pandey@amd.com>; Laurent >> Pinchart <laurent.pinchart@ideasonboard.com>; linux- >> phy@lists.infradead.org >> Cc: Vinod Koul <vkoul@kernel.org>; linux-arm-kernel@lists.infradead.org; >> linux-kernel@vger.kernel.org; Simek, Michal <michal.simek@amd.com>; >> Kishon Vijay Abraham I <kishon@kernel.org> >> Subject: Re: [PATCH v2 4/4] phy: zynqmp: Add debugfs support >> >> On 6/13/24 05:20, Pandey, Radhey Shyam wrote: >> >> -----Original Message----- >> >> From: Sean Anderson <sean.anderson@linux.dev> >> >> Sent: Monday, May 6, 2024 10:31 PM >> >> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>; linux- >> >> phy@lists.infradead.org >> >> Cc: Vinod Koul <vkoul@kernel.org>; linux-arm-kernel@lists.infradead.org; >> >> linux-kernel@vger.kernel.org; Michal Simek <michal.simek@amd.com>; >> >> Kishon Vijay Abraham I <kishon@kernel.org>; Sean Anderson >> >> <sean.anderson@linux.dev> >> >> Subject: [PATCH v2 4/4] phy: zynqmp: Add debugfs support >> >> >> >> Add support for printing some basic status information to debugfs. This >> >> is helpful when debugging phy consumers to make sure they are >> configuring >> >> the phy appropriately. >> >> >> >> Signed-off-by: Sean Anderson <sean.anderson@linux.dev> >> >> --- >> >> >> >> Changes in v2: >> >> - Use debugfs_create_devm_seqfile >> >> >> >> drivers/phy/xilinx/phy-zynqmp.c | 40 >> >> +++++++++++++++++++++++++++++++++ >> >> 1 file changed, 40 insertions(+) >> >> >> >> diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy- >> >> zynqmp.c >> >> index b86fe2a249a8..2520c75a4a77 100644 >> >> --- a/drivers/phy/xilinx/phy-zynqmp.c >> >> +++ b/drivers/phy/xilinx/phy-zynqmp.c >> >> @@ -13,6 +13,7 @@ >> >> */ >> >> >> >> #include <linux/clk.h> >> >> +#include <linux/debugfs.h> >> >> #include <linux/delay.h> >> >> #include <linux/io.h> >> >> #include <linux/kernel.h> >> >> @@ -123,6 +124,15 @@ >> >> #define ICM_PROTOCOL_DP 0x4 >> >> #define ICM_PROTOCOL_SGMII 0x5 >> >> >> >> +static const char *const xpsgtr_icm_str[] = { >> >> + [ICM_PROTOCOL_PD] = "powered down", >> > >> > Protocol saying powered down seems confusing. >> > Should we say None or None(power down)? >> >> Either works I guess. I'm just matching the define. > > None seems fine - we can respin if there are no objections. >> >> >> + [ICM_PROTOCOL_PCIE] = "PCIe", >> >> + [ICM_PROTOCOL_SATA] = "SATA", >> >> + [ICM_PROTOCOL_USB] = "USB", >> >> + [ICM_PROTOCOL_DP] = "DisplayPort", >> >> + [ICM_PROTOCOL_SGMII] = "SGMII", >> >> +}; >> >> + >> >> /* Test Mode common reset control parameters */ >> >> #define TM_CMN_RST 0x10018 >> >> #define TM_CMN_RST_EN 0x1 >> >> @@ -788,6 +798,34 @@ static struct phy *xpsgtr_xlate(struct device *dev, >> >> return ERR_PTR(-EINVAL); >> >> } >> >> >> >> +/* >> >> + * DebugFS >> >> + */ >> >> + >> >> +static int xpsgtr_status_read(struct seq_file *seq, void *data) >> >> +{ >> >> + struct device *dev = seq->private; >> >> + struct xpsgtr_phy *gtr_phy = dev_get_drvdata(dev); >> >> + struct clk *clk; >> >> + u32 pll_status; >> >> + >> >> + mutex_lock(>r_phy->phy->mutex); >> > >> > Do we see any need for this lock? This function simply read hw register >> > /phy members and decodes it. >> >> It's to protect against modifications to gtr_phy->refclk and ->instance. > > Refclock and instance is assigned in xlate which is not protected by any > Lock. Also xpsgtr_phy_init () uses a different gtr_mutex. So want > to understand this phy->mutex need. Ah, well then we should take this lock in xlate. --Sean >> >> These are accessed under lock elsewhere; this is not a fast path and I don't >> want to have to reason about the semantics when using a mutex is definitely >> correct. >> >> --Sean >> >> >> + pll_status = xpsgtr_read_phy(gtr_phy, L0_PLL_STATUS_READ_1); >> >> + clk = gtr_phy->dev->clk[gtr_phy->refclk]; >> >> + >> >> + seq_printf(seq, "Lane: %u\n", gtr_phy->lane); >> >> + seq_printf(seq, "Protocol: %s\n", >> >> + xpsgtr_icm_str[gtr_phy->protocol]); >> >> + seq_printf(seq, "Instance: %u\n", gtr_phy->instance); >> >> + seq_printf(seq, "Reference clock: %u (%pC)\n", gtr_phy->refclk, clk); >> >> + seq_printf(seq, "Reference rate: %lu\n", clk_get_rate(clk)); >> >> + seq_printf(seq, "PLL locked: %s\n", >> >> + pll_status & PLL_STATUS_LOCKED ? "yes" : "no"); >> >> + >> >> + mutex_unlock(>r_phy->phy->mutex); >> >> + return 0; >> >> +} >> >> + >> >> /* >> >> * Power Management >> >> */ >> >> @@ -937,6 +975,8 @@ static int xpsgtr_probe(struct platform_device >> *pdev) >> >> >> >> gtr_phy->phy = phy; >> >> phy_set_drvdata(phy, gtr_phy); >> >> + debugfs_create_devm_seqfile(&phy->dev, "status", phy- >> >> >debugfs, >> >> + xpsgtr_status_read); >> >> } >> >> >> >> /* Register the PHY provider. */ >> >> -- >> >> 2.35.1.1320.gc452695387.dirty >> > >
diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c index b86fe2a249a8..2520c75a4a77 100644 --- a/drivers/phy/xilinx/phy-zynqmp.c +++ b/drivers/phy/xilinx/phy-zynqmp.c @@ -13,6 +13,7 @@ */ #include <linux/clk.h> +#include <linux/debugfs.h> #include <linux/delay.h> #include <linux/io.h> #include <linux/kernel.h> @@ -123,6 +124,15 @@ #define ICM_PROTOCOL_DP 0x4 #define ICM_PROTOCOL_SGMII 0x5 +static const char *const xpsgtr_icm_str[] = { + [ICM_PROTOCOL_PD] = "powered down", + [ICM_PROTOCOL_PCIE] = "PCIe", + [ICM_PROTOCOL_SATA] = "SATA", + [ICM_PROTOCOL_USB] = "USB", + [ICM_PROTOCOL_DP] = "DisplayPort", + [ICM_PROTOCOL_SGMII] = "SGMII", +}; + /* Test Mode common reset control parameters */ #define TM_CMN_RST 0x10018 #define TM_CMN_RST_EN 0x1 @@ -788,6 +798,34 @@ static struct phy *xpsgtr_xlate(struct device *dev, return ERR_PTR(-EINVAL); } +/* + * DebugFS + */ + +static int xpsgtr_status_read(struct seq_file *seq, void *data) +{ + struct device *dev = seq->private; + struct xpsgtr_phy *gtr_phy = dev_get_drvdata(dev); + struct clk *clk; + u32 pll_status; + + mutex_lock(>r_phy->phy->mutex); + pll_status = xpsgtr_read_phy(gtr_phy, L0_PLL_STATUS_READ_1); + clk = gtr_phy->dev->clk[gtr_phy->refclk]; + + seq_printf(seq, "Lane: %u\n", gtr_phy->lane); + seq_printf(seq, "Protocol: %s\n", + xpsgtr_icm_str[gtr_phy->protocol]); + seq_printf(seq, "Instance: %u\n", gtr_phy->instance); + seq_printf(seq, "Reference clock: %u (%pC)\n", gtr_phy->refclk, clk); + seq_printf(seq, "Reference rate: %lu\n", clk_get_rate(clk)); + seq_printf(seq, "PLL locked: %s\n", + pll_status & PLL_STATUS_LOCKED ? "yes" : "no"); + + mutex_unlock(>r_phy->phy->mutex); + return 0; +} + /* * Power Management */ @@ -937,6 +975,8 @@ static int xpsgtr_probe(struct platform_device *pdev) gtr_phy->phy = phy; phy_set_drvdata(phy, gtr_phy); + debugfs_create_devm_seqfile(&phy->dev, "status", phy->debugfs, + xpsgtr_status_read); } /* Register the PHY provider. */
Add support for printing some basic status information to debugfs. This is helpful when debugging phy consumers to make sure they are configuring the phy appropriately. Signed-off-by: Sean Anderson <sean.anderson@linux.dev> --- Changes in v2: - Use debugfs_create_devm_seqfile drivers/phy/xilinx/phy-zynqmp.c | 40 +++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)