Commit 3a6a489f authored by Mark Brown's avatar Mark Brown

Merge remote-tracking branches 'asoc/topic/devm', 'asoc/topic/fsl',...

Merge remote-tracking branches 'asoc/topic/devm', 'asoc/topic/fsl', 'asoc/topic/fsl-esai', 'asoc/topic/fsl-sai', 'asoc/topic/fsl-spdif' and 'asoc/topic/fsl-ssi' into asoc-next
......@@ -7,10 +7,11 @@ codec/DSP interfaces.
Required properties:
- compatible: Compatible list, contains "fsl,vf610-sai".
- compatible: Compatible list, contains "fsl,vf610-sai" or "fsl,imx6sx-sai".
- reg: Offset and length of the register set for the device.
- clocks: Must contain an entry for each entry in clock-names.
- clock-names : Must include the "sai" entry.
- clock-names : Must include the "bus" for register access and "mclk1" "mclk2"
"mclk3" for bit clock and frame clock providing.
- dmas : Generic dma devicetree binding as described in
Documentation/devicetree/bindings/dma/dma.txt.
- dma-names : Two dmas have to be defined, "tx" and "rx".
......@@ -30,8 +31,10 @@ sai2: sai@40031000 {
reg = <0x40031000 0x1000>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_sai2_1>;
clocks = <&clks VF610_CLK_SAI2>;
clock-names = "sai";
clocks = <&clks VF610_CLK_PLATFORM_BUS>,
<&clks VF610_CLK_SAI2>,
<&clks 0>, <&clks 0>;
clock-names = "bus", "mclk1", "mclk2", "mclk3";
dma-names = "tx", "rx";
dmas = <&edma0 0 VF610_EDMA_MUXID0_SAI2_TX>,
<&edma0 0 VF610_EDMA_MUXID0_SAI2_RX>;
......
menu "SoC Audio for Freescale CPUs"
comment "Common SoC Audio options for Freescale CPUs:"
config SND_SOC_FSL_SAI
tristate
tristate "Synchronous Audio Interface (SAI) module support"
select REGMAP_MMIO
select SND_SOC_GENERIC_DMAENGINE_PCM
help
Say Y if you want to add Synchronous Audio Interface (SAI)
support for the Freescale CPUs.
This option is only useful for out-of-tree drivers since
in-tree drivers select it automatically.
config SND_SOC_FSL_SSI
tristate
tristate "Synchronous Serial Interface module support"
select SND_SOC_IMX_PCM_DMA if SND_IMX_SOC != n
select SND_SOC_IMX_PCM_FIQ if SND_IMX_SOC != n && ARCH_MXC
help
Say Y if you want to add Synchronous Serial Interface (SSI)
support for the Freescale CPUs.
This option is only useful for out-of-tree drivers since
in-tree drivers select it automatically.
config SND_SOC_FSL_SPDIF
tristate
tristate "Sony/Philips Digital Interface module support"
select REGMAP_MMIO
select SND_SOC_IMX_PCM_DMA if SND_IMX_SOC != n
select SND_SOC_IMX_PCM_FIQ if SND_IMX_SOC != n && ARCH_MXC
help
Say Y if you want to add Sony/Philips Digital Interface (SPDIF)
support for the Freescale CPUs.
This option is only useful for out-of-tree drivers since
in-tree drivers select it automatically.
config SND_SOC_FSL_ESAI
tristate
tristate "Enhanced Serial Audio Interface (ESAI) module support"
select REGMAP_MMIO
select SND_SOC_FSL_UTILS
help
Say Y if you want to add Enhanced Synchronous Audio Interface
(ESAI) support for the Freescale CPUs.
This option is only useful for out-of-tree drivers since
in-tree drivers select it automatically.
config SND_SOC_FSL_UTILS
tristate
menuconfig SND_POWERPC_SOC
config SND_SOC_IMX_PCM_DMA
tristate
select SND_SOC_GENERIC_DMAENGINE_PCM
config SND_SOC_IMX_AUDMUX
tristate "Digital Audio Mux module support"
help
Say Y if you want to add Digital Audio Mux (AUDMUX) support
for the ARM i.MX CPUs.
This option is only useful for out-of-tree drivers since
in-tree drivers select it automatically.
config SND_POWERPC_SOC
tristate "SoC Audio for Freescale PowerPC CPUs"
depends on FSL_SOC || PPC_MPC52xx
help
Say Y or M if you want to add support for codecs attached to
the PowerPC CPUs.
config SND_IMX_SOC
tristate "SoC Audio for Freescale i.MX CPUs"
depends on ARCH_MXC || COMPILE_TEST
help
Say Y or M if you want to add support for codecs attached to
the i.MX CPUs.
if SND_POWERPC_SOC
config SND_MPC52xx_DMA
......@@ -33,6 +80,8 @@ config SND_MPC52xx_DMA
config SND_SOC_POWERPC_DMA
tristate
comment "SoC Audio support for Freescale PPC boards:"
config SND_SOC_MPC8610_HPCD
tristate "ALSA SoC support for the Freescale MPC8610 HPCD board"
# I2C is necessary for the CS4270 driver
......@@ -110,13 +159,6 @@ config SND_MPC52xx_SOC_EFIKA
endif # SND_POWERPC_SOC
menuconfig SND_IMX_SOC
tristate "SoC Audio for Freescale i.MX CPUs"
depends on ARCH_MXC || COMPILE_TEST
help
Say Y or M if you want to add support for codecs attached to
the i.MX CPUs.
if SND_IMX_SOC
config SND_SOC_IMX_SSI
......@@ -127,12 +169,7 @@ config SND_SOC_IMX_PCM_FIQ
tristate
select FIQ
config SND_SOC_IMX_PCM_DMA
tristate
select SND_SOC_GENERIC_DMAENGINE_PCM
config SND_SOC_IMX_AUDMUX
tristate
comment "SoC Audio support for Freescale i.MX boards:"
config SND_MXC_SOC_WM1133_EV1
tristate "Audio on the i.MX31ADS with WM1133-EV1 fitted"
......@@ -225,3 +262,5 @@ config SND_SOC_IMX_MC13783
select SND_SOC_IMX_PCM_DMA
endif # SND_IMX_SOC
endmenu
......@@ -12,7 +12,8 @@ obj-$(CONFIG_SND_SOC_P1022_RDK) += snd-soc-p1022-rdk.o
# Freescale SSI/DMA/SAI/SPDIF Support
snd-soc-fsl-sai-objs := fsl_sai.o
snd-soc-fsl-ssi-objs := fsl_ssi.o
snd-soc-fsl-ssi-y := fsl_ssi.o
snd-soc-fsl-ssi-$(CONFIG_DEBUG_FS) += fsl_ssi_dbg.o
snd-soc-fsl-spdif-objs := fsl_spdif.o
snd-soc-fsl-esai-objs := fsl_esai.o
snd-soc-fsl-utils-objs := fsl_utils.o
......
......@@ -39,6 +39,8 @@
* @fifo_depth: depth of tx/rx FIFO
* @slot_width: width of each DAI slot
* @hck_rate: clock rate of desired HCKx clock
* @sck_rate: clock rate of desired SCKx clock
* @hck_dir: the direction of HCKx pads
* @sck_div: if using PSR/PM dividers for SCKx clock
* @slave_mode: if fully using DAI slave mode
* @synchronous: if using tx/rx synchronous mode
......@@ -55,6 +57,8 @@ struct fsl_esai {
u32 fifo_depth;
u32 slot_width;
u32 hck_rate[2];
u32 sck_rate[2];
bool hck_dir[2];
bool sck_div[2];
bool slave_mode;
bool synchronous;
......@@ -209,8 +213,13 @@ static int fsl_esai_set_dai_sysclk(struct snd_soc_dai *dai, int clk_id,
struct clk *clksrc = esai_priv->extalclk;
bool tx = clk_id <= ESAI_HCKT_EXTAL;
bool in = dir == SND_SOC_CLOCK_IN;
u32 ret, ratio, ecr = 0;
u32 ratio, ecr = 0;
unsigned long clk_rate;
int ret;
/* Bypass divider settings if the requirement doesn't change */
if (freq == esai_priv->hck_rate[tx] && dir == esai_priv->hck_dir[tx])
return 0;
/* sck_div can be only bypassed if ETO/ERO=0 and SNC_SOC_CLOCK_OUT */
esai_priv->sck_div[tx] = true;
......@@ -277,6 +286,7 @@ static int fsl_esai_set_dai_sysclk(struct snd_soc_dai *dai, int clk_id,
esai_priv->sck_div[tx] = false;
out:
esai_priv->hck_dir[tx] = dir;
esai_priv->hck_rate[tx] = freq;
regmap_update_bits(esai_priv->regmap, REG_ESAI_ECR,
......@@ -294,9 +304,10 @@ static int fsl_esai_set_bclk(struct snd_soc_dai *dai, bool tx, u32 freq)
struct fsl_esai *esai_priv = snd_soc_dai_get_drvdata(dai);
u32 hck_rate = esai_priv->hck_rate[tx];
u32 sub, ratio = hck_rate / freq;
int ret;
/* Don't apply for fully slave mode*/
if (esai_priv->slave_mode)
/* Don't apply for fully slave mode or unchanged bclk */
if (esai_priv->slave_mode || esai_priv->sck_rate[tx] == freq)
return 0;
if (ratio * freq > hck_rate)
......@@ -319,8 +330,15 @@ static int fsl_esai_set_bclk(struct snd_soc_dai *dai, bool tx, u32 freq)
return -EINVAL;
}
return fsl_esai_divisor_cal(dai, tx, ratio, true,
ret = fsl_esai_divisor_cal(dai, tx, ratio, true,
esai_priv->sck_div[tx] ? 0 : ratio);
if (ret)
return ret;
/* Save current bclk rate */
esai_priv->sck_rate[tx] = freq;
return 0;
}
static int fsl_esai_set_dai_tdm_slot(struct snd_soc_dai *dai, u32 tx_mask,
......@@ -439,8 +457,8 @@ static int fsl_esai_set_dai_fmt(struct snd_soc_dai *dai, unsigned int fmt)
static int fsl_esai_startup(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)
{
int ret;
struct fsl_esai *esai_priv = snd_soc_dai_get_drvdata(dai);
int ret;
/*
* Some platforms might use the same bit to gate all three or two of
......@@ -492,7 +510,8 @@ static int fsl_esai_hw_params(struct snd_pcm_substream *substream,
bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
u32 width = snd_pcm_format_width(params_format(params));
u32 channels = params_channels(params);
u32 bclk, mask, val, ret;
u32 bclk, mask, val;
int ret;
bclk = params_rate(params) * esai_priv->slot_width * 2;
......@@ -822,6 +841,7 @@ static int fsl_esai_probe(struct platform_device *pdev)
static const struct of_device_id fsl_esai_dt_ids[] = {
{ .compatible = "fsl,imx35-esai", },
{ .compatible = "fsl,vf610-esai", },
{}
};
MODULE_DEVICE_TABLE(of, fsl_esai_dt_ids);
......
This diff is collapsed.
......@@ -35,6 +35,16 @@
#define FSL_SAI_RFR 0xc0 /* SAI Receive FIFO */
#define FSL_SAI_RMR 0xe0 /* SAI Receive Mask */
#define FSL_SAI_xCSR(tx) (tx ? FSL_SAI_TCSR : FSL_SAI_RCSR)
#define FSL_SAI_xCR1(tx) (tx ? FSL_SAI_TCR1 : FSL_SAI_RCR1)
#define FSL_SAI_xCR2(tx) (tx ? FSL_SAI_TCR2 : FSL_SAI_RCR2)
#define FSL_SAI_xCR3(tx) (tx ? FSL_SAI_TCR3 : FSL_SAI_RCR3)
#define FSL_SAI_xCR4(tx) (tx ? FSL_SAI_TCR4 : FSL_SAI_RCR4)
#define FSL_SAI_xCR5(tx) (tx ? FSL_SAI_TCR5 : FSL_SAI_RCR5)
#define FSL_SAI_xDR(tx) (tx ? FSL_SAI_TDR : FSL_SAI_RDR)
#define FSL_SAI_xFR(tx) (tx ? FSL_SAI_TFR : FSL_SAI_RFR)
#define FSL_SAI_xMR(tx) (tx ? FSL_SAI_TMR : FSL_SAI_RMR)
/* SAI Transmit/Recieve Control Register */
#define FSL_SAI_CSR_TERE BIT(31)
#define FSL_SAI_CSR_FR BIT(25)
......@@ -48,6 +58,7 @@
#define FSL_SAI_CSR_FWF BIT(17)
#define FSL_SAI_CSR_FRF BIT(16)
#define FSL_SAI_CSR_xIE_SHIFT 8
#define FSL_SAI_CSR_xIE_MASK (0x1f << FSL_SAI_CSR_xIE_SHIFT)
#define FSL_SAI_CSR_WSIE BIT(12)
#define FSL_SAI_CSR_SEIE BIT(11)
#define FSL_SAI_CSR_FEIE BIT(10)
......@@ -108,6 +119,8 @@
#define FSL_SAI_CLK_MAST2 2
#define FSL_SAI_CLK_MAST3 3
#define FSL_SAI_MCLK_MAX 3
/* SAI data transfer numbers per DMA request */
#define FSL_SAI_MAXBURST_TX 6
#define FSL_SAI_MAXBURST_RX 6
......@@ -115,10 +128,13 @@
struct fsl_sai {
struct platform_device *pdev;
struct regmap *regmap;
struct clk *bus_clk;
struct clk *mclk_clk[FSL_SAI_MCLK_MAX];
bool big_endian_regs;
bool big_endian_data;
bool is_dsp_mode;
bool sai_on_imx;
struct snd_dmaengine_dai_dma_data dma_params_rx;
struct snd_dmaengine_dai_dma_data dma_params_tx;
......
This diff is collapsed.
......@@ -143,20 +143,22 @@ enum spdif_gainsel {
#define INT_RXFIFO_FUL (1 << 0)
/* SPDIF Clock register */
#define STC_SYSCLK_DIV_OFFSET 11
#define STC_SYSCLK_DIV_MASK (0x1ff << STC_SYSCLK_DIV_OFFSET)
#define STC_SYSCLK_DIV(x) ((((x) - 1) << STC_SYSCLK_DIV_OFFSET) & STC_SYSCLK_DIV_MASK)
#define STC_SYSCLK_DF_OFFSET 11
#define STC_SYSCLK_DF_MASK (0x1ff << STC_SYSCLK_DF_OFFSET)
#define STC_SYSCLK_DF(x) ((((x) - 1) << STC_SYSCLK_DF_OFFSET) & STC_SYSCLK_DF_MASK)
#define STC_TXCLK_SRC_OFFSET 8
#define STC_TXCLK_SRC_MASK (0x7 << STC_TXCLK_SRC_OFFSET)
#define STC_TXCLK_SRC_SET(x) ((x << STC_TXCLK_SRC_OFFSET) & STC_TXCLK_SRC_MASK)
#define STC_TXCLK_ALL_EN_OFFSET 7
#define STC_TXCLK_ALL_EN_MASK (1 << STC_TXCLK_ALL_EN_OFFSET)
#define STC_TXCLK_ALL_EN (1 << STC_TXCLK_ALL_EN_OFFSET)
#define STC_TXCLK_DIV_OFFSET 0
#define STC_TXCLK_DIV_MASK (0x7ff << STC_TXCLK_DIV_OFFSET)
#define STC_TXCLK_DIV(x) ((((x) - 1) << STC_TXCLK_DIV_OFFSET) & STC_TXCLK_DIV_MASK)
#define STC_TXCLK_DF_OFFSET 0
#define STC_TXCLK_DF_MASK (0x7ff << STC_TXCLK_DF_OFFSET)
#define STC_TXCLK_DF(x) ((((x) - 1) << STC_TXCLK_DF_OFFSET) & STC_TXCLK_DF_MASK)
#define STC_TXCLK_SRC_MAX 8
#define STC_TXCLK_SPDIF_ROOT 1
/* SPDIF tx rate */
enum spdif_txrate {
SPDIF_TXRATE_32000 = 0,
......
This diff is collapsed.
......@@ -39,6 +39,7 @@ struct ccsr_ssi {
__be32 saccdis; /* 0x.0058 - SSI AC97 Channel Disable Register */
};
#define CCSR_SSI_SCR_SYNC_TX_FS 0x00001000
#define CCSR_SSI_SCR_RFR_CLK_DIS 0x00000800
#define CCSR_SSI_SCR_TFR_CLK_DIS 0x00000400
#define CCSR_SSI_SCR_TCH_EN 0x00000100
......@@ -206,5 +207,64 @@ struct ccsr_ssi {
#define CCSR_SSI_SACNT_FV 0x00000002
#define CCSR_SSI_SACNT_AC97EN 0x00000001
#endif
struct device;
#if IS_ENABLED(CONFIG_DEBUG_FS)
struct fsl_ssi_dbg {
struct dentry *dbg_dir;
struct dentry *dbg_stats;
struct {
unsigned int rfrc;
unsigned int tfrc;
unsigned int cmdau;
unsigned int cmddu;
unsigned int rxt;
unsigned int rdr1;
unsigned int rdr0;
unsigned int tde1;
unsigned int tde0;
unsigned int roe1;
unsigned int roe0;
unsigned int tue1;
unsigned int tue0;
unsigned int tfs;
unsigned int rfs;
unsigned int tls;
unsigned int rls;
unsigned int rff1;
unsigned int rff0;
unsigned int tfe1;
unsigned int tfe0;
} stats;
};
void fsl_ssi_dbg_isr(struct fsl_ssi_dbg *ssi_dbg, u32 sisr);
int fsl_ssi_debugfs_create(struct fsl_ssi_dbg *ssi_dbg, struct device *dev);
void fsl_ssi_debugfs_remove(struct fsl_ssi_dbg *ssi_dbg);
#else
struct fsl_ssi_dbg {
};
static inline void fsl_ssi_dbg_isr(struct fsl_ssi_dbg *stats, u32 sisr)
{
}
static inline int fsl_ssi_debugfs_create(struct fsl_ssi_dbg *ssi_dbg,
struct device *dev)
{
return 0;
}
static inline void fsl_ssi_debugfs_remove(struct fsl_ssi_dbg *ssi_dbg)
{
}
#endif /* ! IS_ENABLED(CONFIG_DEBUG_FS) */
#endif
/*
* Freescale SSI ALSA SoC Digital Audio Interface (DAI) debugging functions
*
* Copyright 2014 Markus Pargmann <mpa@pengutronix.de>, Pengutronix
*
* Splitted from fsl_ssi.c
*
* This file is licensed under the terms of the GNU General Public License
* version 2. This program is licensed "as is" without any warranty of any
* kind, whether express or implied.
*/
#include <linux/debugfs.h>
#include <linux/device.h>
#include <linux/kernel.h>
#include "fsl_ssi.h"
void fsl_ssi_dbg_isr(struct fsl_ssi_dbg *dbg, u32 sisr)
{
if (sisr & CCSR_SSI_SISR_RFRC)
dbg->stats.rfrc++;
if (sisr & CCSR_SSI_SISR_TFRC)
dbg->stats.tfrc++;
if (sisr & CCSR_SSI_SISR_CMDAU)
dbg->stats.cmdau++;
if (sisr & CCSR_SSI_SISR_CMDDU)
dbg->stats.cmddu++;
if (sisr & CCSR_SSI_SISR_RXT)
dbg->stats.rxt++;
if (sisr & CCSR_SSI_SISR_RDR1)
dbg->stats.rdr1++;
if (sisr & CCSR_SSI_SISR_RDR0)
dbg->stats.rdr0++;
if (sisr & CCSR_SSI_SISR_TDE1)
dbg->stats.tde1++;
if (sisr & CCSR_SSI_SISR_TDE0)
dbg->stats.tde0++;
if (sisr & CCSR_SSI_SISR_ROE1)
dbg->stats.roe1++;
if (sisr & CCSR_SSI_SISR_ROE0)
dbg->stats.roe0++;
if (sisr & CCSR_SSI_SISR_TUE1)
dbg->stats.tue1++;
if (sisr & CCSR_SSI_SISR_TUE0)
dbg->stats.tue0++;
if (sisr & CCSR_SSI_SISR_TFS)
dbg->stats.tfs++;
if (sisr & CCSR_SSI_SISR_RFS)
dbg->stats.rfs++;
if (sisr & CCSR_SSI_SISR_TLS)
dbg->stats.tls++;
if (sisr & CCSR_SSI_SISR_RLS)
dbg->stats.rls++;
if (sisr & CCSR_SSI_SISR_RFF1)
dbg->stats.rff1++;
if (sisr & CCSR_SSI_SISR_RFF0)
dbg->stats.rff0++;
if (sisr & CCSR_SSI_SISR_TFE1)
dbg->stats.tfe1++;
if (sisr & CCSR_SSI_SISR_TFE0)
dbg->stats.tfe0++;
}
/* Show the statistics of a flag only if its interrupt is enabled. The
* compiler will optimze this code to a no-op if the interrupt is not
* enabled.
*/
#define SIER_SHOW(flag, name) \
do { \
if (CCSR_SSI_SIER_##flag) \
seq_printf(s, #name "=%u\n", ssi_dbg->stats.name); \
} while (0)
/**
* fsl_sysfs_ssi_show: display SSI statistics
*
* Display the statistics for the current SSI device. To avoid confusion,
* we only show those counts that are enabled.
*/
static int fsl_ssi_stats_show(struct seq_file *s, void *unused)
{
struct fsl_ssi_dbg *ssi_dbg = s->private;
SIER_SHOW(RFRC_EN, rfrc);
SIER_SHOW(TFRC_EN, tfrc);
SIER_SHOW(CMDAU_EN, cmdau);
SIER_SHOW(CMDDU_EN, cmddu);
SIER_SHOW(RXT_EN, rxt);
SIER_SHOW(RDR1_EN, rdr1);
SIER_SHOW(RDR0_EN, rdr0);
SIER_SHOW(TDE1_EN, tde1);
SIER_SHOW(TDE0_EN, tde0);
SIER_SHOW(ROE1_EN, roe1);
SIER_SHOW(ROE0_EN, roe0);
SIER_SHOW(TUE1_EN, tue1);
SIER_SHOW(TUE0_EN, tue0);
SIER_SHOW(TFS_EN, tfs);
SIER_SHOW(RFS_EN, rfs);
SIER_SHOW(TLS_EN, tls);
SIER_SHOW(RLS_EN, rls);
SIER_SHOW(RFF1_EN, rff1);
SIER_SHOW(RFF0_EN, rff0);
SIER_SHOW(TFE1_EN, tfe1);
SIER_SHOW(TFE0_EN, tfe0);
return 0;
}
static int fsl_ssi_stats_open(struct inode *inode, struct file *file)
{
return single_open(file, fsl_ssi_stats_show, inode->i_private);
}
static const struct file_operations fsl_ssi_stats_ops = {
.open = fsl_ssi_stats_open,
.read = seq_read,
.llseek = seq_lseek,
.release = single_release,
};
int fsl_ssi_debugfs_create(struct fsl_ssi_dbg *ssi_dbg, struct device *dev)
{
ssi_dbg->dbg_dir = debugfs_create_dir(dev_name(dev), NULL);
if (!ssi_dbg->dbg_dir)
return -ENOMEM;
ssi_dbg->dbg_stats = debugfs_create_file("stats", S_IRUGO,
ssi_dbg->dbg_dir, ssi_dbg, &fsl_ssi_stats_ops);
if (!ssi_dbg->dbg_stats) {
debugfs_remove(ssi_dbg->dbg_dir);
return -ENOMEM;
}
return 0;
}
void fsl_ssi_debugfs_remove(struct fsl_ssi_dbg *ssi_dbg)
{
debugfs_remove(ssi_dbg->dbg_stats);
debugfs_remove(ssi_dbg->dbg_dir);
}
......@@ -40,7 +40,6 @@ static const struct snd_pcm_hardware imx_pcm_hardware = {
SNDRV_PCM_INFO_MMAP_VALID |
SNDRV_PCM_INFO_PAUSE |
SNDRV_PCM_INFO_RESUME,
.formats = SNDRV_PCM_FMTBIT_S16_LE,
.buffer_bytes_max = IMX_SSI_DMABUF_SIZE,
.period_bytes_min = 128,
.period_bytes_max = 65535, /* Limited by SDMA engine */
......
......@@ -85,6 +85,7 @@ int devm_snd_soc_register_platform(struct device *dev,
return ret;
}
EXPORT_SYMBOL_GPL(devm_snd_soc_register_platform);
static void devm_card_release(struct device *dev, void *res)
{
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment