[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Libcdio-devel] Read Sub-channel changes
From: |
Thomas Schmitt |
Subject: |
Re: [Libcdio-devel] Read Sub-channel changes |
Date: |
Wed, 08 Jun 2016 22:47:25 +0200 |
Hi,
> Again, we should not contemplate changing something that exists
I am well aware of the goal to keep API and ABI stable.
Further i do not want to break things without a good excuse prepared.
> It would be fine to create a function that runs a mmc command and then
> issues a sense command. Since it issues the mmc command it knows what
> command it ran.
I understand you are in favor of this alternative:
> > - Move sense code evaluation and reporting to functions which know
> > the mmc_cdb_t. This knowledge is currently quite low-level and very
> > volatile.
So i will move the call of mmc_evaluate_last_sense() from
mmc_get_mcn_isrc_private() to mmc_read_subchannel().
This prevents my plan to let error indicating sense data act like
missing MCVAL/TCVAL bit. These bits are known only in
mmc_get_mcn_isrc_private().
In mmc_read_subchannel() i can only return something like DRIVER_OP_ERROR.
Is DRIVER_OP_ERROR the appropriate indicator for invalid reply data ?
A report (deliberately provoked by wrong track number 4) now looks like this
++ WARN: READ_SUBCHANNEL (CDB: 42 00 40 03 00 00 04 00 04 00 )
++ WARN: [5 24 00] : Illegal Request, Invalid field in cdb
(Shall i downgrade it to INFO ?
Ignorable problems will be reported by INFO.)
This is the call in mmc_read_subchannel which causes the warnings:
if(mmc_evaluate_last_sense(p_cdio, &cdb, CDIO_MMC_EVAL_SENSE_WARN)
> CDIO_MMC_EVAL_SENSE_IGN)
i_status = DRIVER_OP_ERROR;
It should be executed in low level MMC functions immediately after
i_status = MMC_RUN_CMD(SCSI_MMC_DATA_READ, i_timeout_ms);
This is not ideal because DRIVER_OP_ERROR could override other errors.
But if sense data was evaluated only if (i_status == DRIVER_OP_SUCCESS)
then ioctl(CDROM_SEND_PACKET) would obscure the detailed error message
by its errno == EIO.
I guess this is the appropriate companion macro for MMC_RUN_CMD
#define MMC_EVAL_SENSE(i_status, report_mode) \
if(mmc_evaluate_last_sense(p_cdio, &cdb, report_mode) \
> CDIO_MMC_EVAL_SENSE_IGN) \
i_status = DRIVER_OP_ERROR;
which reduces the footprint of above evaluation to
MMC_EVAL_SENSE(i_status, CDIO_MMC_EVAL_SENSE_WARN)
(Shall i leave out the ';' in the macro so that its invocation needs one ?)
---------------------------------------------------------------------------
If this mechanism gets accepted then one would next have to sprinkle its
use all over those parts of lib/driver/mmc/ which call op.run_mmc_cmd().
Of course one may only change those code parts which one is able to test.
Have a nice day :)
Thomas
- Re: [Libcdio-devel] Place for test documentation, (continued)
- Re: [Libcdio-devel] Place for test documentation, Leon Merten Lohse, 2016/06/04
- Re: [Libcdio-devel] Read Sub-channel changes, Thomas Schmitt, 2016/06/04
- Re: [Libcdio-devel] Read Sub-channel changes, Thomas Schmitt, 2016/06/05
- Re: [Libcdio-devel] Read Sub-channel changes, Rocky Bernstein, 2016/06/05
- Re: [Libcdio-devel] Read Sub-channel changes, Thomas Schmitt, 2016/06/06
- Re: [Libcdio-devel] Read Sub-channel changes, Rocky Bernstein, 2016/06/07
- Re: [Libcdio-devel] Read Sub-channel changes, Thomas Schmitt, 2016/06/07
- Re: [Libcdio-devel] Read Sub-channel changes, Rocky Bernstein, 2016/06/07
- Re: [Libcdio-devel] Read Sub-channel changes, Thomas Schmitt, 2016/06/08
- Re: [Libcdio-devel] Read Sub-channel changes, Rocky Bernstein, 2016/06/08
- Re: [Libcdio-devel] Read Sub-channel changes,
Thomas Schmitt <=
- Re: [Libcdio-devel] Read Sub-channel changes, Rocky Bernstein, 2016/06/08
- Re: [Libcdio-devel] Read Sub-channel changes, Thomas Schmitt, 2016/06/09
- Re: [Libcdio-devel] Read Sub-channel changes, Rocky Bernstein, 2016/06/09
- Re: [Libcdio-devel] Read Sub-channel changes, Rocky Bernstein, 2016/06/09
- Re: [Libcdio-devel] Read Sub-channel changes, Thomas Schmitt, 2016/06/09
- Re: [Libcdio-devel] Read Sub-channel changes, Rocky Bernstein, 2016/06/09
- Re: [Libcdio-devel] Read Sub-channel changes, Thomas Schmitt, 2016/06/11
- Re: [Libcdio-devel] Read Sub-channel changes, Thomas Schmitt, 2016/06/11