Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   "Viorel Suman (OSS)" <viorel.suman@oss.nxp.com>
To:     Mark Brown <broonie@kernel.org>,
        "Viorel Suman (OSS)" <viorel.suman@oss.nxp.com>
CC:     Liam Girdwood <lgirdwood@gmail.com>,
        Rob Herring <robh+dt@kernel.org>,
        Jaroslav Kysela <perex@perex.cz>,
        Takashi Iwai <tiwai@suse.com>, Timur Tabi <timur@kernel.org>,
        Nicolin Chen <nicoleotsuka@gmail.com>,
        Xiubo Li <Xiubo.Lee@gmail.com>,
        Fabio Estevam <festevam@gmail.com>,
        Shengjiu Wang <shengjiu.wang@gmail.com>,
        Philipp Zabel <p.zabel@pengutronix.de>,
        Matthias Schiffer <matthias.schiffer@ew.tq-group.com>,
        Cosmin-Gabriel Samoila <cosmin.samoila@nxp.com>,
        "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
        dl-linux-imx <linux-imx@nxp.com>,
        Viorel Suman <viorel.suman@gmail.com>
Subject: RE: [PATCH 1/2] ASoC: fsl_xcvr: Add XCVR ASoC CPU DAI driver
Thread-Topic: [PATCH 1/2] ASoC: fsl_xcvr: Add XCVR ASoC CPU DAI driver
Thread-Index: AQHWjApN0GGys4KDOUW4RdRuflK20als26gAgAGlwnCAAATfAIAAA9/g
Date:   Fri, 18 Sep 2020 15:33:56 +0000
Message-ID: <VI1PR0401MB2272D66A58EDE0E5C0C8F32D923F0@VI1PR0401MB2272.eurprd04.prod.outlook.com>
References: <1600247876-8013-1-git-send-email-viorel.suman@oss.nxp.com>
 <1600247876-8013-2-git-send-email-viorel.suman@oss.nxp.com>
 <20200917135306.GF4755@sirena.org.uk>
 <VI1PR0401MB22726CC099099547A0502C27923F0@VI1PR0401MB2272.eurprd04.prod.outlook.com>
 <20200918152004.GJ5703@sirena.org.uk>
In-Reply-To: <20200918152004.GJ5703@sirena.org.uk>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-messagesentrepresentingtype: 1
x-originating-ip: [86.127.156.60]
x-ms-publictraffictype: Email
x-ms-office365-filtering-ht: Tenant
x-ms-office365-filtering-correlation-id: 818b1fec-2111-48e9-b3b2-08d85be841de
x-ms-traffictypediagnostic: VI1PR04MB5359:
x-ms-exchange-sharedmailbox-routingagent-processed: True
x-ms-exchange-transport-forked: True
x-microsoft-antispam-prvs: <VI1PR04MB5359BE2F51190012598500E4D33F0@VI1PR04MB5359.eurprd04.prod.outlook.com>
x-ms-oob-tlc-oobclassifiers: OLM:10000;
x-ms-exchange-senderadcheck: 1
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info: lzzAop5mmEHd8+9dqvcyPeePvKsAfjVUC4MNiAmv1P403OXSg0EAVr75Ash1puAHV/4hFKfocbdHbrOLSe4fg2EV5ePzEknD9h2x9emIDVWDhk4rjY8S9eag+fXVaNrN6YdZnR3hMfitWRJvh6jCauuapvjwVViBBPc5ywrZqamqRf+4dEfSDw2RG/6YOMaxuWfoKG+DYwpDuerA7Y4ZJZazyMX2c5kTn5SeNz5dOArrMb0Y0GrqiA6xEkaMN7nK9sABHVVAY2lmFSd5FDjnhnCmnqKMAqcX5EK+UJGaWANn7rls2vey/Q1w4s8XwjB8pTcuANlZSkGXUvgv8T/lp10Ksa6fO+0XL+5A9mksvVK0vwxXYgzkzjhXmulbK/eN
x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:VI1PR0401MB2272.eurprd04.prod.outlook.com;PTR:;CAT:NONE;SFS:(4636009)(346002)(136003)(39860400002)(366004)(376002)(396003)(66446008)(64756008)(4326008)(7696005)(186003)(8676002)(86362001)(33656002)(9686003)(316002)(478600001)(26005)(55016002)(6506007)(110136005)(5660300002)(54906003)(66556008)(66476007)(66946007)(7416002)(52536014)(83380400001)(71200400001)(2906002)(8936002)(76116006);DIR:OUT;SFP:1101;
x-ms-exchange-antispam-messagedata: 7nwAk6G7SsNmKvlMDEK8tPk7burm/+bfodLUtWn9LMU/dFuQovCsX1ilX6hUHRcrvSKG7tr0ndh7Z3qdlZhaVVtjmHeaN/+alg6R67Iwa0hO19zwOhtzIfIEiQIRsdy0CY5pTZ7dwby+dgkeu97nBCI1FSFULZvLUs79cqcP7jH4LsVASjm5MiiHjrSCRnqfLbiGS7A1psv1gpZP81+NgspdAMKo6Crk48IcNAbG2a/6qR6PyB8a1ddwLCD07yjnUMQ1Bx3teIz0HFTRn/DS4Lq9mJzNcnyuYFkslv/FB8TIzhFWdMic2POszo39YU/7hpTe8tuArtD02wsJ+QLTW6fy94ZIe5OI13p9D2TD/+fHurE5qODLsTkn9E2AafaKqnXo7l4kCPqq8UpG1+884LVM8Kep44TyhqF8XRJCP9GATN3hr4HnARxn7NuFYV18goMVhf44cZi7ItPqHZstmYtIAexLs9ysXUXBgdqY8w/XMs8D8OacBBXCqCS2A0BB5fkoVoWOeedSxUGQGJP0VE4UgSibOYiy5Hd5Ri8OTuLoEcNbVKqG86n4XcNOJk3N9hF/0tiY/Oiz2TtEnuaVqwHQiwgPWvvIB1C3HrAkurePdY4Njz4V0wjNxdK1Jyljpth73srUAyLuouEmaRIY9g==
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: oss.nxp.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: VI1PR0401MB2272.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 818b1fec-2111-48e9-b3b2-08d85be841de
X-MS-Exchange-CrossTenant-originalarrivaltime: 18 Sep 2020 15:33:56.6890
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: OI+7A30krELdCsKyekaIO6Gc8FTJP3ZizEuqo+HSn61P3xlr11d/XTVP9kKEisx1wOYRPYaOsHysr9+aB5TnRQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB5359
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> On Fri, Sep 18, 2020 at 03:02:39PM +0000, Viorel Suman (OSS) wrote:
>=20
> Please fix your mail client to word wrap within paragraphs at something
> substantially less than 80 columns.  Doing this makes your messages much =
easier
> to read and reply to.
>=20

My bad, will do.

> > > > +	regmap_read(regmap, FSL_XCVR_EXT_ISR, &isr);
> > > > +	regmap_write(regmap, FSL_XCVR_EXT_ISR_CLR, isr);
>=20
> > > This will unconditionally clear any interrupts, even those we don't
> > > understand - it might be better to only clear bits that are
> > > supported so the IRQ core can complain if there's something unexpecte=
d
> showing up.
>=20
> > The ARM core registers itself in "fsl_xcvr_prepare" (the code below) ju=
st for a
> subset of all supported interrupts:
> > =3D=3D=3D=3D=3D
> > 	ret =3D regmap_update_bits(xcvr->regmap, FSL_XCVR_EXT_IER0,
> > 				 FSL_XCVR_IRQ_EARC_ALL,
> FSL_XCVR_IRQ_EARC_ALL); =3D=3D=3D=3D=3D
> > FSL_XCVR_IRQ_EARC_ALL - this mask represents all the interrupts we are
> > interested in and we handle in interrupt handler, But this is just a su=
bset of all
> interrupts the M0+ core is able to assert. Not very intuitive, I think I =
need to
> reword it somehow.
>=20
> That's not the issue, the issue is that if we get into the ISR we just ac=
k all the bits
> that are flagged by the hardware regardless of if we actually handled the=
m.  This
> won't work if there are ever systems that share the interrupt and it work=
s
> against safety/debugging features that the interrupt has in case somethin=
g goes
> wrong and we get spurious interrupts.
>=20

Thank you for explanation, will fix it in next version.

> > > > +	if (isr & FSL_XCVR_IRQ_FIFO_UOFL_ERR)
> > > > +		dev_dbg(dev, "RX/TX FIFO full/empty\n");
>=20
> > > Should this be dev_err()?
>=20
> > The interrupt may be asserted right before DMA starts to fill the TX FI=
FO if I
> recall correctly.
> > I've added it just to debug the IP behavior, will check and change it t=
o err it in
> next version if it is the case.
>=20
> If it does come up normally then a comment or something to explain why th=
is
> happens normally would probably be good.

Sure, ok.