2023-01-19 22:28:22

by Andy Shevchenko

Subject: [PATCH v1 1/9] pktcdvd: Get rid of custom printing macros

We may use traditional dev_*() macros instead of custom ones
provided by the driver.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/block/pktcdvd.c | 247 ++++++++++++++++++-----------------
include/linux/pktcdvd.h | 1 -
include/uapi/linux/pktcdvd.h | 1 +
3 files changed, 130 insertions(+), 119 deletions(-)

diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 2f1a92509271..0ec8dc8ee5ed 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -72,22 +72,6 @@

#define DRIVER_NAME "pktcdvd"

-#define pkt_err(pd, fmt, ...) \
- pr_err("%s: " fmt, pd->name, ##__VA_ARGS__)
-#define pkt_notice(pd, fmt, ...) \
- pr_notice("%s: " fmt, pd->name, ##__VA_ARGS__)
-#define pkt_info(pd, fmt, ...) \
- pr_info("%s: " fmt, pd->name, ##__VA_ARGS__)
-
-#define pkt_dbg(level, pd, fmt, ...) \
-do { \
- if (level == 2 && PACKET_DEBUG >= 2) \
- pr_notice("%s: %s():" fmt, \
- pd->name, __func__, ##__VA_ARGS__); \
- else if (level == 1 && PACKET_DEBUG >= 1) \
- pr_notice("%s: " fmt, pd->name, ##__VA_ARGS__); \
-} while (0)
-
#define MAX_SPEED 0xffff

static DEFINE_MUTEX(pktcdvd_mutex);
@@ -318,7 +302,7 @@ static void pkt_sysfs_dev_new(struct pktcdvd_device *pd)
if (class_pktcdvd) {
pd->dev = device_create_with_groups(class_pktcdvd, NULL,
MKDEV(0, 0), pd, pkt_groups,
- "%s", pd->name);
+ "%s", pd->disk->disk_name);
if (IS_ERR(pd->dev))
pd->dev = NULL;
}
@@ -354,7 +338,7 @@ static ssize_t device_map_show(struct class *c, struct class_attribute *attr,
if (!pd)
continue;
n += sprintf(data+n, "%s %u:%u %u:%u\n",
- pd->name,
+ pd->disk->disk_name,
MAJOR(pd->pkt_dev), MINOR(pd->pkt_dev),
MAJOR(pd->bdev->bd_dev),
MINOR(pd->bdev->bd_dev));
@@ -467,7 +451,7 @@ static void pkt_debugfs_dev_new(struct pktcdvd_device *pd)
{
if (!pkt_debugfs_root)
return;
- pd->dfs_d_root = debugfs_create_dir(pd->name, pkt_debugfs_root);
+ pd->dfs_d_root = debugfs_create_dir(pd->disk->disk_name, pkt_debugfs_root);
if (!pd->dfs_d_root)
return;

@@ -501,9 +485,11 @@ static void pkt_debugfs_cleanup(void)

static void pkt_bio_finished(struct pktcdvd_device *pd)
{
+ struct device *ddev = disk_to_dev(pd->disk);
+
BUG_ON(atomic_read(&pd->cdrw.pending_bios) <= 0);
if (atomic_dec_and_test(&pd->cdrw.pending_bios)) {
- pkt_dbg(2, pd, "queue empty\n");
+ dev_dbg(ddev, "queue empty\n");
atomic_set(&pd->iosched.attention, 1);
wake_up(&pd->wqueue);
}
@@ -734,15 +720,16 @@ static const char *sense_key_string(__u8 index)
static void pkt_dump_sense(struct pktcdvd_device *pd,
struct packet_command *cgc)
{
+ struct device *ddev = disk_to_dev(pd->disk);
struct scsi_sense_hdr *sshdr = cgc->sshdr;

if (sshdr)
- pkt_err(pd, "%*ph - sense %02x.%02x.%02x (%s)\n",
+ dev_err(ddev, "%*ph - sense %02x.%02x.%02x (%s)\n",
CDROM_PACKET_SIZE, cgc->cmd,
sshdr->sense_key, sshdr->asc, sshdr->ascq,
sense_key_string(sshdr->sense_key));
else
- pkt_err(pd, "%*ph - no sense\n", CDROM_PACKET_SIZE, cgc->cmd);
+ dev_err(ddev, "%*ph - no sense\n", CDROM_PACKET_SIZE, cgc->cmd);
}

/*
@@ -826,6 +813,7 @@ static void pkt_queue_bio(struct pktcdvd_device *pd, struct bio *bio)
*/
static void pkt_iosched_process_queue(struct pktcdvd_device *pd)
{
+ struct device *ddev = disk_to_dev(pd->disk);

if (atomic_read(&pd->iosched.attention) == 0)
return;
@@ -853,7 +841,7 @@ static void pkt_iosched_process_queue(struct pktcdvd_device *pd)
need_write_seek = 0;
if (need_write_seek && reads_queued) {
if (atomic_read(&pd->cdrw.pending_bios) > 0) {
- pkt_dbg(2, pd, "write, waiting\n");
+ dev_dbg(ddev, "write, waiting\n");
break;
}
pkt_flush_cache(pd);
@@ -862,7 +850,7 @@ static void pkt_iosched_process_queue(struct pktcdvd_device *pd)
} else {
if (!reads_queued && writes_queued) {
if (atomic_read(&pd->cdrw.pending_bios) > 0) {
- pkt_dbg(2, pd, "read, waiting\n");
+ dev_dbg(ddev, "read, waiting\n");
break;
}
pd->iosched.writing = 1;
@@ -909,6 +897,8 @@ static void pkt_iosched_process_queue(struct pktcdvd_device *pd)
*/
static int pkt_set_segment_merging(struct pktcdvd_device *pd, struct request_queue *q)
{
+ struct device *ddev = disk_to_dev(pd->disk);
+
if ((pd->settings.size << 9) / CD_FRAMESIZE
<= queue_max_segments(q)) {
/*
@@ -925,7 +915,7 @@ static int pkt_set_segment_merging(struct pktcdvd_device *pd, struct request_que
set_bit(PACKET_MERGE_SEGS, &pd->flags);
return 0;
} else {
- pkt_err(pd, "cdrom max_phys_segments too small\n");
+ dev_err(ddev, "cdrom max_phys_segments too small\n");
return -EIO;
}
}
@@ -936,7 +926,7 @@ static void pkt_end_io_read(struct bio *bio)
struct pktcdvd_device *pd = pkt->pd;
BUG_ON(!pd);

- pkt_dbg(2, pd, "bio=%p sec0=%llx sec=%llx err=%d\n",
+ dev_dbg(disk_to_dev(pd->disk), "bio=%p sec0=%llx sec=%llx err=%d\n",
bio, (unsigned long long)pkt->sector,
(unsigned long long)bio->bi_iter.bi_sector, bio->bi_status);

@@ -956,7 +946,7 @@ static void pkt_end_io_packet_write(struct bio *bio)
struct pktcdvd_device *pd = pkt->pd;
BUG_ON(!pd);

- pkt_dbg(2, pd, "id=%d, err=%d\n", pkt->id, bio->bi_status);
+ dev_dbg(disk_to_dev(pd->disk), "id=%d, err=%d\n", pkt->id, bio->bi_status);

pd->stats.pkt_ended++;

@@ -972,6 +962,7 @@ static void pkt_end_io_packet_write(struct bio *bio)
*/
static void pkt_gather_data(struct pktcdvd_device *pd, struct packet_data *pkt)
{
+ struct device *ddev = disk_to_dev(pd->disk);
int frames_read = 0;
struct bio *bio;
int f;
@@ -1000,8 +991,7 @@ static void pkt_gather_data(struct pktcdvd_device *pd, struct packet_data *pkt)
spin_unlock(&pkt->lock);

if (pkt->cache_valid) {
- pkt_dbg(2, pd, "zone %llx cached\n",
- (unsigned long long)pkt->sector);
+ dev_dbg(ddev, "zone %llx cached\n", (unsigned long long)pkt->sector);
goto out_account;
}

@@ -1022,8 +1012,8 @@ static void pkt_gather_data(struct pktcdvd_device *pd, struct packet_data *pkt)

p = (f * CD_FRAMESIZE) / PAGE_SIZE;
offset = (f * CD_FRAMESIZE) % PAGE_SIZE;
- pkt_dbg(2, pd, "Adding frame %d, page:%p offs:%d\n",
- f, pkt->pages[p], offset);
+ dev_dbg(ddev, "Adding frame %d, page:%p offs:%d\n", f,
+ pkt->pages[p], offset);
if (!bio_add_page(bio, pkt->pages[p], CD_FRAMESIZE, offset))
BUG();

@@ -1033,8 +1023,8 @@ static void pkt_gather_data(struct pktcdvd_device *pd, struct packet_data *pkt)
}

out_account:
- pkt_dbg(2, pd, "need %d frames for zone %llx\n",
- frames_read, (unsigned long long)pkt->sector);
+ dev_dbg(ddev, "need %d frames for zone %llx\n", frames_read,
+ (unsigned long long)pkt->sector);
pd->stats.pkt_started++;
pd->stats.secs_rg += frames_read * (CD_FRAMESIZE >> 9);
}
@@ -1068,17 +1058,18 @@ static void pkt_put_packet_data(struct pktcdvd_device *pd, struct packet_data *p
}
}

-static inline void pkt_set_state(struct packet_data *pkt, enum packet_data_state state)
+static inline void pkt_set_state(struct device *ddev, struct packet_data *pkt,
+ enum packet_data_state state)
{
-#if PACKET_DEBUG > 1
static const char *state_name[] = {
"IDLE", "WAITING", "READ_WAIT", "WRITE_WAIT", "RECOVERY", "FINISHED"
};
enum packet_data_state old_state = pkt->state;
- pkt_dbg(2, pd, "pkt %2d : s=%6llx %s -> %s\n",
+
+ dev_dbg(ddev, "pkt %2d : s=%6llx %s -> %s\n",
pkt->id, (unsigned long long)pkt->sector,
state_name[old_state], state_name[state]);
-#endif
+
pkt->state = state;
}

@@ -1088,6 +1079,7 @@ static inline void pkt_set_state(struct packet_data *pkt, enum packet_data_state
*/
static int pkt_handle_queue(struct pktcdvd_device *pd)
{
+ struct device *ddev = disk_to_dev(pd->disk);
struct packet_data *pkt, *p;
struct bio *bio = NULL;
sector_t zone = 0; /* Suppress gcc warning */
@@ -1097,7 +1089,7 @@ static int pkt_handle_queue(struct pktcdvd_device *pd)
atomic_set(&pd->scan_queue, 0);

if (list_empty(&pd->cdrw.pkt_free_list)) {
- pkt_dbg(2, pd, "no pkt\n");
+ dev_dbg(ddev, "no pkt\n");
return 0;
}

@@ -1134,7 +1126,7 @@ static int pkt_handle_queue(struct pktcdvd_device *pd)
}
spin_unlock(&pd->lock);
if (!bio) {
- pkt_dbg(2, pd, "no bio\n");
+ dev_dbg(ddev, "no bio\n");
return 0;
}

@@ -1150,12 +1142,13 @@ static int pkt_handle_queue(struct pktcdvd_device *pd)
* to this packet.
*/
spin_lock(&pd->lock);
- pkt_dbg(2, pd, "looking for zone %llx\n", (unsigned long long)zone);
+ dev_dbg(ddev, "looking for zone %llx\n", (unsigned long long)zone);
while ((node = pkt_rbtree_find(pd, zone)) != NULL) {
+ sector_t tmp = get_zone(node->bio->bi_iter.bi_sector, pd);
+
bio = node->bio;
- pkt_dbg(2, pd, "found zone=%llx\n", (unsigned long long)
- get_zone(bio->bi_iter.bi_sector, pd));
- if (get_zone(bio->bi_iter.bi_sector, pd) != zone)
+ dev_dbg(ddev, "found zone=%llx\n", (unsigned long long)tmp);
+ if (tmp != zone)
break;
pkt_rbtree_erase(pd, node);
spin_lock(&pkt->lock);
@@ -1174,7 +1167,7 @@ static int pkt_handle_queue(struct pktcdvd_device *pd)
spin_unlock(&pd->lock);

pkt->sleep_time = max(PACKET_WAIT_TIME, 1);
- pkt_set_state(pkt, PACKET_WAITING_STATE);
+ pkt_set_state(ddev, pkt, PACKET_WAITING_STATE);
atomic_set(&pkt->run_sm, 1);

spin_lock(&pd->cdrw.active_list_lock);
@@ -1226,6 +1219,7 @@ static void bio_list_copy_data(struct bio *dst, struct bio *src)
*/
static void pkt_start_write(struct pktcdvd_device *pd, struct packet_data *pkt)
{
+ struct device *ddev = disk_to_dev(pd->disk);
int f;

bio_init(pkt->w_bio, pd->bdev, pkt->w_bio->bi_inline_vecs, pkt->frames,
@@ -1242,7 +1236,7 @@ static void pkt_start_write(struct pktcdvd_device *pd, struct packet_data *pkt)
if (!bio_add_page(pkt->w_bio, page, CD_FRAMESIZE, offset))
BUG();
}
- pkt_dbg(2, pd, "vcnt=%d\n", pkt->w_bio->bi_vcnt);
+ dev_dbg(ddev, "vcnt=%d\n", pkt->w_bio->bi_vcnt);

/*
* Fill-in bvec with data from orig_bios.
@@ -1250,11 +1244,11 @@ static void pkt_start_write(struct pktcdvd_device *pd, struct packet_data *pkt)
spin_lock(&pkt->lock);
bio_list_copy_data(pkt->w_bio, pkt->orig_bios.head);

- pkt_set_state(pkt, PACKET_WRITE_WAIT_STATE);
+ pkt_set_state(ddev, pkt, PACKET_WRITE_WAIT_STATE);
spin_unlock(&pkt->lock);

- pkt_dbg(2, pd, "Writing %d frames for zone %llx\n",
- pkt->write_size, (unsigned long long)pkt->sector);
+ dev_dbg(ddev, "Writing %d frames for zone %llx\n", pkt->write_size,
+ (unsigned long long)pkt->sector);

if (test_bit(PACKET_MERGE_SEGS, &pd->flags) || (pkt->write_size < pkt->frames))
pkt->cache_valid = 1;
@@ -1282,7 +1276,9 @@ static void pkt_finish_packet(struct packet_data *pkt, blk_status_t status)

static void pkt_run_state_machine(struct pktcdvd_device *pd, struct packet_data *pkt)
{
- pkt_dbg(2, pd, "pkt %d\n", pkt->id);
+ struct device *ddev = disk_to_dev(pd->disk);
+
+ dev_dbg(ddev, "pkt %d\n", pkt->id);

for (;;) {
switch (pkt->state) {
@@ -1292,7 +1288,7 @@ static void pkt_run_state_machine(struct pktcdvd_device *pd, struct packet_data

pkt->sleep_time = 0;
pkt_gather_data(pd, pkt);
- pkt_set_state(pkt, PACKET_READ_WAIT_STATE);
+ pkt_set_state(ddev, pkt, PACKET_READ_WAIT_STATE);
break;

case PACKET_READ_WAIT_STATE:
@@ -1300,7 +1296,7 @@ static void pkt_run_state_machine(struct pktcdvd_device *pd, struct packet_data
return;

if (atomic_read(&pkt->io_errors) > 0) {
- pkt_set_state(pkt, PACKET_RECOVERY_STATE);
+ pkt_set_state(ddev, pkt, PACKET_RECOVERY_STATE);
} else {
pkt_start_write(pd, pkt);
}
@@ -1311,15 +1307,15 @@ static void pkt_run_state_machine(struct pktcdvd_device *pd, struct packet_data
return;

if (!pkt->w_bio->bi_status) {
- pkt_set_state(pkt, PACKET_FINISHED_STATE);
+ pkt_set_state(ddev, pkt, PACKET_FINISHED_STATE);
} else {
- pkt_set_state(pkt, PACKET_RECOVERY_STATE);
+ pkt_set_state(ddev, pkt, PACKET_RECOVERY_STATE);
}
break;

case PACKET_RECOVERY_STATE:
- pkt_dbg(2, pd, "No recovery possible\n");
- pkt_set_state(pkt, PACKET_FINISHED_STATE);
+ dev_dbg(ddev, "No recovery possible\n");
+ pkt_set_state(ddev, pkt, PACKET_FINISHED_STATE);
break;

case PACKET_FINISHED_STATE:
@@ -1335,6 +1331,7 @@ static void pkt_run_state_machine(struct pktcdvd_device *pd, struct packet_data

static void pkt_handle_packets(struct pktcdvd_device *pd)
{
+ struct device *ddev = disk_to_dev(pd->disk);
struct packet_data *pkt, *next;

/*
@@ -1355,7 +1352,7 @@ static void pkt_handle_packets(struct pktcdvd_device *pd)
if (pkt->state == PACKET_FINISHED_STATE) {
list_del(&pkt->list);
pkt_put_packet_data(pd, pkt);
- pkt_set_state(pkt, PACKET_IDLE_STATE);
+ pkt_set_state(ddev, pkt, PACKET_IDLE_STATE);
atomic_set(&pd->scan_queue, 1);
}
}
@@ -1384,7 +1381,9 @@ static void pkt_count_states(struct pktcdvd_device *pd, int *states)
static int kcdrwd(void *foobar)
{
struct pktcdvd_device *pd = foobar;
+ struct device *ddev = disk_to_dev(pd->disk);
struct packet_data *pkt;
+ int states[PACKET_NUM_STATES];
long min_sleep_time, residue;

set_user_nice(current, MIN_NICE);
@@ -1415,13 +1414,9 @@ static int kcdrwd(void *foobar)
goto work_to_do;

/* Otherwise, go to sleep */
- if (PACKET_DEBUG > 1) {
- int states[PACKET_NUM_STATES];
- pkt_count_states(pd, states);
- pkt_dbg(2, pd, "i:%d ow:%d rw:%d ww:%d rec:%d fin:%d\n",
- states[0], states[1], states[2],
- states[3], states[4], states[5]);
- }
+ pkt_count_states(pd, states);
+ dev_dbg(ddev, "i:%d ow:%d rw:%d ww:%d rec:%d fin:%d\n",
+ states[0], states[1], states[2], states[3], states[4], states[5]);

min_sleep_time = MAX_SCHEDULE_TIMEOUT;
list_for_each_entry(pkt, &pd->cdrw.pkt_active_list, list) {
@@ -1429,9 +1424,9 @@ static int kcdrwd(void *foobar)
min_sleep_time = pkt->sleep_time;
}

- pkt_dbg(2, pd, "sleeping\n");
+ dev_dbg(ddev, "sleeping\n");
residue = schedule_timeout(min_sleep_time);
- pkt_dbg(2, pd, "wake up\n");
+ dev_dbg(ddev, "wake up\n");

/* make swsusp happy with our thread */
try_to_freeze();
@@ -1479,7 +1474,7 @@ static int kcdrwd(void *foobar)

static void pkt_print_settings(struct pktcdvd_device *pd)
{
- pkt_info(pd, "%s packets, %u blocks, Mode-%c disc\n",
+ dev_info(disk_to_dev(pd->disk), "%s packets, %u blocks, Mode-%c disc\n",
pd->settings.fp ? "Fixed" : "Variable",
pd->settings.size >> 2,
pd->settings.block_mode == 8 ? '1' : '2');
@@ -1607,6 +1602,7 @@ static noinline_for_stack int pkt_get_last_written(struct pktcdvd_device *pd,
*/
static noinline_for_stack int pkt_set_write_settings(struct pktcdvd_device *pd)
{
+ struct device *ddev = disk_to_dev(pd->disk);
struct packet_command cgc;
struct scsi_sense_hdr sshdr;
write_param_page *wp;
@@ -1673,7 +1669,7 @@ static noinline_for_stack int pkt_set_write_settings(struct pktcdvd_device *pd)
/*
* paranoia
*/
- pkt_err(pd, "write mode wrong %d\n", wp->data_block_type);
+ dev_err(ddev, "write mode wrong %d\n", wp->data_block_type);
return 1;
}
wp->packet_size = cpu_to_be32(pd->settings.size >> 2);
@@ -1694,6 +1690,8 @@ static noinline_for_stack int pkt_set_write_settings(struct pktcdvd_device *pd)
*/
static int pkt_writable_track(struct pktcdvd_device *pd, track_information *ti)
{
+ struct device *ddev = disk_to_dev(pd->disk);
+
switch (pd->mmc3_profile) {
case 0x1a: /* DVD+RW */
case 0x12: /* DVD-RAM */
@@ -1718,7 +1716,7 @@ static int pkt_writable_track(struct pktcdvd_device *pd, track_information *ti)
if (ti->rt == 1 && ti->blank == 0)
return 1;

- pkt_err(pd, "bad state %d-%d-%d\n", ti->rt, ti->blank, ti->packet);
+ dev_err(ddev, "bad state %d-%d-%d\n", ti->rt, ti->blank, ti->packet);
return 0;
}

@@ -1727,6 +1725,8 @@ static int pkt_writable_track(struct pktcdvd_device *pd, track_information *ti)
*/
static int pkt_writable_disc(struct pktcdvd_device *pd, disc_information *di)
{
+ struct device *ddev = disk_to_dev(pd->disk);
+
switch (pd->mmc3_profile) {
case 0x0a: /* CD-RW */
case 0xffff: /* MMC3 not supported */
@@ -1736,8 +1736,7 @@ static int pkt_writable_disc(struct pktcdvd_device *pd, disc_information *di)
case 0x12: /* DVD-RAM */
return 1;
default:
- pkt_dbg(2, pd, "Wrong disc profile (%x)\n",
- pd->mmc3_profile);
+ dev_dbg(ddev, "Wrong disc profile (%x)\n", pd->mmc3_profile);
return 0;
}

@@ -1746,22 +1745,22 @@ static int pkt_writable_disc(struct pktcdvd_device *pd, disc_information *di)
* but i'm not sure, should we leave this to user apps? probably.
*/
if (di->disc_type == 0xff) {
- pkt_notice(pd, "unknown disc - no track?\n");
+ dev_notice(ddev, "unknown disc - no track?\n");
return 0;
}

if (di->disc_type != 0x20 && di->disc_type != 0) {
- pkt_err(pd, "wrong disc type (%x)\n", di->disc_type);
+ dev_err(ddev, "wrong disc type (%x)\n", di->disc_type);
return 0;
}

if (di->erasable == 0) {
- pkt_notice(pd, "disc not erasable\n");
+ dev_err(ddev, "disc not erasable\n");
return 0;
}

if (di->border_status == PACKET_SESSION_RESERVED) {
- pkt_err(pd, "can't write to last track (reserved)\n");
+ dev_err(ddev, "can't write to last track (reserved)\n");
return 0;
}

@@ -1770,6 +1769,7 @@ static int pkt_writable_disc(struct pktcdvd_device *pd, disc_information *di)

static noinline_for_stack int pkt_probe_settings(struct pktcdvd_device *pd)
{
+ struct device *ddev = disk_to_dev(pd->disk);
struct packet_command cgc;
unsigned char buf[12];
disc_information di;
@@ -1787,7 +1787,7 @@ static noinline_for_stack int pkt_probe_settings(struct pktcdvd_device *pd)

ret = pkt_get_disc_info(pd, &di);
if (ret) {
- pkt_err(pd, "failed get_disc\n");
+ dev_err(ddev, "failed get_disc\n");
return ret;
}

@@ -1799,12 +1799,12 @@ static noinline_for_stack int pkt_probe_settings(struct pktcdvd_device *pd)
track = 1; /* (di.last_track_msb << 8) | di.last_track_lsb; */
ret = pkt_get_track_info(pd, track, 1, &ti);
if (ret) {
- pkt_err(pd, "failed get_track\n");
+ dev_err(ddev, "failed get_track\n");
return ret;
}

if (!pkt_writable_track(pd, &ti)) {
- pkt_err(pd, "can't write to this track\n");
+ dev_err(ddev, "can't write to this track\n");
return -EROFS;
}

@@ -1814,11 +1814,11 @@ static noinline_for_stack int pkt_probe_settings(struct pktcdvd_device *pd)
*/
pd->settings.size = be32_to_cpu(ti.fixed_packet_size) << 2;
if (pd->settings.size == 0) {
- pkt_notice(pd, "detected zero packet size!\n");
+ dev_notice(ddev, "detected zero packet size!\n");
return -ENXIO;
}
if (pd->settings.size > PACKET_MAX_SECTORS) {
- pkt_err(pd, "packet size is too big\n");
+ dev_err(ddev, "packet size is too big\n");
return -EROFS;
}
pd->settings.fp = ti.fp;
@@ -1860,7 +1860,7 @@ static noinline_for_stack int pkt_probe_settings(struct pktcdvd_device *pd)
pd->settings.block_mode = PACKET_BLOCK_MODE2;
break;
default:
- pkt_err(pd, "unknown data mode\n");
+ dev_err(ddev, "unknown data mode\n");
return -EROFS;
}
return 0;
@@ -1872,6 +1872,7 @@ static noinline_for_stack int pkt_probe_settings(struct pktcdvd_device *pd)
static noinline_for_stack int pkt_write_caching(struct pktcdvd_device *pd,
int set)
{
+ struct device *ddev = disk_to_dev(pd->disk);
struct packet_command cgc;
struct scsi_sense_hdr sshdr;
unsigned char buf[64];
@@ -1895,10 +1896,10 @@ static noinline_for_stack int pkt_write_caching(struct pktcdvd_device *pd,
cgc.buflen = cgc.cmd[8] = 2 + ((buf[0] << 8) | (buf[1] & 0xff));
ret = pkt_mode_select(pd, &cgc);
if (ret) {
- pkt_err(pd, "write caching control failed\n");
+ dev_err(ddev, "write caching control failed\n");
pkt_dump_sense(pd, &cgc);
} else if (!ret && set)
- pkt_notice(pd, "enabled write caching\n");
+ dev_notice(ddev, "enabled write caching\n");
return ret;
}

@@ -1979,6 +1980,7 @@ static char us_clv_to_speed[16] = {
static noinline_for_stack int pkt_media_speed(struct pktcdvd_device *pd,
unsigned *speed)
{
+ struct device *ddev = disk_to_dev(pd->disk);
struct packet_command cgc;
struct scsi_sense_hdr sshdr;
unsigned char buf[64];
@@ -2013,11 +2015,11 @@ static noinline_for_stack int pkt_media_speed(struct pktcdvd_device *pd,
}

if (!(buf[6] & 0x40)) {
- pkt_notice(pd, "disc type is not CD-RW\n");
+ dev_notice(ddev, "disc type is not CD-RW\n");
return 1;
}
if (!(buf[6] & 0x4)) {
- pkt_notice(pd, "A1 values on media are not valid, maybe not CDRW?\n");
+ dev_notice(ddev, "A1 values on media are not valid, maybe not CDRW?\n");
return 1;
}

@@ -2037,25 +2039,26 @@ static noinline_for_stack int pkt_media_speed(struct pktcdvd_device *pd,
*speed = us_clv_to_speed[sp];
break;
default:
- pkt_notice(pd, "unknown disc sub-type %d\n", st);
+ dev_notice(ddev, "unknown disc sub-type %d\n", st);
return 1;
}
if (*speed) {
- pkt_info(pd, "maximum media speed: %d\n", *speed);
+ dev_info(ddev, "maximum media speed: %d\n", *speed);
return 0;
} else {
- pkt_notice(pd, "unknown speed %d for sub-type %d\n", sp, st);
+ dev_notice(ddev, "unknown speed %d for sub-type %d\n", sp, st);
return 1;
}
}

static noinline_for_stack int pkt_perform_opc(struct pktcdvd_device *pd)
{
+ struct device *ddev = disk_to_dev(pd->disk);
struct packet_command cgc;
struct scsi_sense_hdr sshdr;
int ret;

- pkt_dbg(2, pd, "Performing OPC\n");
+ dev_dbg(ddev, "Performing OPC\n");

init_cdrom_command(&cgc, NULL, 0, CGC_DATA_NONE);
cgc.sshdr = &sshdr;
@@ -2070,18 +2073,19 @@ static noinline_for_stack int pkt_perform_opc(struct pktcdvd_device *pd)

static int pkt_open_write(struct pktcdvd_device *pd)
{
+ struct device *ddev = disk_to_dev(pd->disk);
int ret;
unsigned int write_speed, media_write_speed, read_speed;

ret = pkt_probe_settings(pd);
if (ret) {
- pkt_dbg(2, pd, "failed probe\n");
+ dev_dbg(ddev, "failed probe\n");
return ret;
}

ret = pkt_set_write_settings(pd);
if (ret) {
- pkt_dbg(1, pd, "failed saving write settings\n");
+ dev_notice(ddev, "failed saving write settings\n");
return -EIO;
}

@@ -2094,30 +2098,29 @@ static int pkt_open_write(struct pktcdvd_device *pd)
case 0x13: /* DVD-RW */
case 0x1a: /* DVD+RW */
case 0x12: /* DVD-RAM */
- pkt_dbg(1, pd, "write speed %ukB/s\n", write_speed);
+ dev_notice(ddev, "write speed %ukB/s\n", write_speed);
break;
default:
ret = pkt_media_speed(pd, &media_write_speed);
if (ret)
media_write_speed = 16;
write_speed = min(write_speed, media_write_speed * 177);
- pkt_dbg(1, pd, "write speed %ux\n", write_speed / 176);
+ dev_notice(ddev, "write speed %ux\n", write_speed / 176);
break;
}
read_speed = write_speed;

ret = pkt_set_speed(pd, write_speed, read_speed);
if (ret) {
- pkt_dbg(1, pd, "couldn't set write speed\n");
+ dev_notice(ddev, "couldn't set write speed\n");
return -EIO;
}
pd->write_speed = write_speed;
pd->read_speed = read_speed;

ret = pkt_perform_opc(pd);
- if (ret) {
- pkt_dbg(1, pd, "Optimum Power Calibration failed\n");
- }
+ if (ret)
+ dev_notice(ddev, "Optimum Power Calibration failed\n");

return 0;
}
@@ -2127,6 +2130,7 @@ static int pkt_open_write(struct pktcdvd_device *pd)
*/
static int pkt_open_dev(struct pktcdvd_device *pd, fmode_t write)
{
+ struct device *ddev = disk_to_dev(pd->disk);
int ret;
long lba;
struct request_queue *q;
@@ -2145,7 +2149,7 @@ static int pkt_open_dev(struct pktcdvd_device *pd, fmode_t write)

ret = pkt_get_last_written(pd, &lba);
if (ret) {
- pkt_err(pd, "pkt_get_last_written failed\n");
+ dev_err(ddev, "pkt_get_last_written failed\n");
goto out_putdev;
}

@@ -2174,11 +2178,11 @@ static int pkt_open_dev(struct pktcdvd_device *pd, fmode_t write)

if (write) {
if (!pkt_grow_pktlist(pd, CONFIG_CDROM_PKTCDVD_BUFFERS)) {
- pkt_err(pd, "not enough memory for buffers\n");
+ dev_err(ddev, "not enough memory for buffers\n");
ret = -ENOMEM;
goto out_putdev;
}
- pkt_info(pd, "%lukB available on disc\n", lba << 1);
+ dev_info(ddev, "%lukB available on disc\n", lba << 1);
}

return 0;
@@ -2195,8 +2199,10 @@ static int pkt_open_dev(struct pktcdvd_device *pd, fmode_t write)
*/
static void pkt_release_dev(struct pktcdvd_device *pd, int flush)
{
+ struct device *ddev = disk_to_dev(pd->disk);
+
if (flush && pkt_flush_cache(pd))
- pkt_dbg(1, pd, "not flushing cache\n");
+ dev_notice(ddev, "not flushing cache\n");

pkt_lock_door(pd, 0);

@@ -2397,13 +2403,14 @@ static void pkt_make_request_write(struct request_queue *q, struct bio *bio)
static void pkt_submit_bio(struct bio *bio)
{
struct pktcdvd_device *pd = bio->bi_bdev->bd_disk->queue->queuedata;
+ struct device *ddev = disk_to_dev(pd->disk);
struct bio *split;

bio = bio_split_to_limits(bio);
if (!bio)
return;

- pkt_dbg(2, pd, "start = %6llx stop = %6llx\n",
+ dev_dbg(ddev, "start = %6llx stop = %6llx\n",
(unsigned long long)bio->bi_iter.bi_sector,
(unsigned long long)bio_end_sector(bio));

@@ -2416,13 +2423,13 @@ static void pkt_submit_bio(struct bio *bio)
}

if (!test_bit(PACKET_WRITABLE, &pd->flags)) {
- pkt_notice(pd, "WRITE for ro device (%llu)\n",
+ dev_notice(ddev, "WRITE for ro device (%llu)\n",
(unsigned long long)bio->bi_iter.bi_sector);
goto end_io;
}

if (!bio->bi_iter.bi_size || (bio->bi_iter.bi_size % CD_FRAMESIZE)) {
- pkt_err(pd, "wrong bio size\n");
+ dev_err(ddev, "wrong bio size\n");
goto end_io;
}

@@ -2464,7 +2471,7 @@ static int pkt_seq_show(struct seq_file *m, void *p)
char *msg;
int states[PACKET_NUM_STATES];

- seq_printf(m, "Writer %s mapped to %pg:\n", pd->name, pd->bdev);
+ seq_printf(m, "Writer %s mapped to %pg:\n", pd->disk->disk_name, pd->bdev);

seq_printf(m, "\nSettings:\n");
seq_printf(m, "\tpacket size:\t\t%dkB\n", pd->settings.size / 2);
@@ -2520,12 +2527,13 @@ static int pkt_seq_show(struct seq_file *m, void *p)

static int pkt_new_dev(struct pktcdvd_device *pd, dev_t dev)
{
+ struct device *ddev = disk_to_dev(pd->disk);
int i;
struct block_device *bdev;
struct scsi_device *sdev;

if (pd->pkt_dev == dev) {
- pkt_err(pd, "recursive setup not allowed\n");
+ dev_err(ddev, "recursive setup not allowed\n");
return -EBUSY;
}
for (i = 0; i < MAX_WRITERS; i++) {
@@ -2533,11 +2541,11 @@ static int pkt_new_dev(struct pktcdvd_device *pd, dev_t dev)
if (!pd2)
continue;
if (pd2->bdev->bd_dev == dev) {
- pkt_err(pd, "%pg already setup\n", pd2->bdev);
+ dev_err(ddev, "%pg already setup\n", pd2->bdev);
return -EBUSY;
}
if (pd2->pkt_dev == dev) {
- pkt_err(pd, "can't chain pktcdvd devices\n");
+ dev_err(ddev, "can't chain pktcdvd devices\n");
return -EBUSY;
}
}
@@ -2561,14 +2569,14 @@ static int pkt_new_dev(struct pktcdvd_device *pd, dev_t dev)
pkt_init_queue(pd);

atomic_set(&pd->cdrw.pending_bios, 0);
- pd->cdrw.thread = kthread_run(kcdrwd, pd, "%s", pd->name);
+ pd->cdrw.thread = kthread_run(kcdrwd, pd, "%s", pd->disk->disk_name);
if (IS_ERR(pd->cdrw.thread)) {
- pkt_err(pd, "can't start kernel thread\n");
+ dev_err(ddev, "can't start kernel thread\n");
goto out_mem;
}

- proc_create_single_data(pd->name, 0, pkt_proc, pkt_seq_show, pd);
- pkt_dbg(1, pd, "writer mapped to %pg\n", bdev);
+ proc_create_single_data(pd->disk->disk_name, 0, pkt_proc, pkt_seq_show, pd);
+ dev_notice(ddev, "writer mapped to %pg\n", bdev);
return 0;

out_mem:
@@ -2581,10 +2589,10 @@ static int pkt_new_dev(struct pktcdvd_device *pd, dev_t dev)
static int pkt_ioctl(struct block_device *bdev, fmode_t mode, unsigned int cmd, unsigned long arg)
{
struct pktcdvd_device *pd = bdev->bd_disk->private_data;
+ struct device *ddev = disk_to_dev(pd->disk);
int ret;

- pkt_dbg(2, pd, "cmd %x, dev %d:%d\n",
- cmd, MAJOR(bdev->bd_dev), MINOR(bdev->bd_dev));
+ dev_dbg(ddev, "cmd %x, dev %d:%d\n", cmd, MAJOR(bdev->bd_dev), MINOR(bdev->bd_dev));

mutex_lock(&pktcdvd_mutex);
switch (cmd) {
@@ -2610,7 +2618,7 @@ static int pkt_ioctl(struct block_device *bdev, fmode_t mode, unsigned int cmd,
ret = bdev->bd_disk->fops->ioctl(bdev, mode, cmd, arg);
break;
default:
- pkt_dbg(2, pd, "Unknown ioctl (%x)\n", cmd);
+ dev_dbg(ddev, "Unknown ioctl (%x)\n", cmd);
ret = -ENOTTY;
}
mutex_unlock(&pktcdvd_mutex);
@@ -2688,7 +2696,6 @@ static int pkt_setup_dev(dev_t dev, dev_t* pkt_dev)
spin_lock_init(&pd->iosched.lock);
bio_list_init(&pd->iosched.read_queue);
bio_list_init(&pd->iosched.write_queue);
- sprintf(pd->name, DRIVER_NAME"%d", idx);
init_waitqueue_head(&pd->wqueue);
pd->bio_queue = RB_ROOT;

@@ -2705,7 +2712,7 @@ static int pkt_setup_dev(dev_t dev, dev_t* pkt_dev)
disk->minors = 1;
disk->fops = &pktcdvd_ops;
disk->flags = GENHD_FL_REMOVABLE | GENHD_FL_NO_PART;
- strcpy(disk->disk_name, pd->name);
+ snprintf(disk->disk_name, sizeof(disk->disk_name), DRIVER_NAME"%d", idx);
disk->private_data = pd;

pd->pkt_dev = MKDEV(pktdev_major, idx);
@@ -2747,6 +2754,7 @@ static int pkt_setup_dev(dev_t dev, dev_t* pkt_dev)
static int pkt_remove_dev(dev_t pkt_dev)
{
struct pktcdvd_device *pd;
+ struct device *ddev;
int idx;
int ret = 0;

@@ -2767,6 +2775,9 @@ static int pkt_remove_dev(dev_t pkt_dev)
ret = -EBUSY;
goto out;
}
+
+ ddev = disk_to_dev(pd->disk);
+
if (!IS_ERR(pd->cdrw.thread))
kthread_stop(pd->cdrw.thread);

@@ -2777,8 +2788,8 @@ static int pkt_remove_dev(dev_t pkt_dev)

blkdev_put(pd->bdev, FMODE_READ | FMODE_NDELAY);

- remove_proc_entry(pd->name, pkt_proc);
- pkt_dbg(1, pd, "writer unmapped\n");
+ remove_proc_entry(pd->disk->disk_name, pkt_proc);
+ dev_notice(ddev, "writer unmapped\n");

del_gendisk(pd->disk);
put_disk(pd->disk);
diff --git a/include/linux/pktcdvd.h b/include/linux/pktcdvd.h
index f9c5ac80d59b..80cb00db42a4 100644
--- a/include/linux/pktcdvd.h
+++ b/include/linux/pktcdvd.h
@@ -156,7 +156,6 @@ struct pktcdvd_device
{
struct block_device *bdev; /* dev attached */
dev_t pkt_dev; /* our dev */
- char name[20];
struct packet_settings settings;
struct packet_stats stats;
int refcnt; /* Open count */
diff --git a/include/uapi/linux/pktcdvd.h b/include/uapi/linux/pktcdvd.h
index 9cbb55d21c94..69121ca7065c 100644
--- a/include/uapi/linux/pktcdvd.h
+++ b/include/uapi/linux/pktcdvd.h
@@ -16,6 +16,7 @@
#include <linux/types.h>

/*
+ * UNUSED:
* 1 for normal debug messages, 2 is very verbose. 0 to turn it off.
*/
#define PACKET_DEBUG 1
--
2.39.0

2023-01-19 22:28:40

by Andy Shevchenko

Subject: [PATCH v1 6/9] pktcdvd: Use DEFINE_SHOW_ATTRIBUTE() to simplify code

Use DEFINE_SHOW_ATTRIBUTE() helper macro to simplify the code.
No functional change.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/block/pktcdvd.c | 23 +++--------------------
1 file changed, 3 insertions(+), 20 deletions(-)

diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index a53d21dbf2ab..2c5441303e55 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -504,24 +504,7 @@ static int pkt_seq_show(struct seq_file *m, void *p)
pd->write_congestion_on);
return 0;
}
-
-static int pkt_debugfs_seq_show(struct seq_file *m, void *p)
-{
- return pkt_seq_show(m, p);
-}
-
-static int pkt_debugfs_fops_open(struct inode *inode, struct file *file)
-{
- return single_open(file, pkt_debugfs_seq_show, inode->i_private);
-}
-
-static const struct file_operations debug_fops = {
- .open = pkt_debugfs_fops_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release = single_release,
- .owner = THIS_MODULE,
-};
+DEFINE_SHOW_ATTRIBUTE(pkt_seq);

static void pkt_debugfs_dev_new(struct pktcdvd_device *pd)
{
@@ -531,8 +514,8 @@ static void pkt_debugfs_dev_new(struct pktcdvd_device *pd)
if (!pd->dfs_d_root)
return;

- pd->dfs_f_info = debugfs_create_file("info", 0444,
- pd->dfs_d_root, pd, &debug_fops);
+ pd->dfs_f_info = debugfs_create_file("info", 0444, pd->dfs_d_root,
+ pd, &pkt_seq_fops);
}

static void pkt_debugfs_dev_remove(struct pktcdvd_device *pd)
--
2.39.0

2023-01-19 22:29:28

by Andy Shevchenko

Subject: [PATCH v1 3/9] pktcdvd: use sysfs_emit() to instead of scnprintf()

Follow the advice of the Documentation/filesystems/sysfs.rst and show()
should only use sysfs_emit() or sysfs_emit_at() when formatting the
value to be returned to user space.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/block/pktcdvd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index ad4336ae9927..98a5c388eabf 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -339,7 +339,7 @@ static ssize_t device_map_show(struct class *c, struct class_attribute *attr,
struct pktcdvd_device *pd = pkt_devs[idx];
if (!pd)
continue;
- n += sprintf(data+n, "%s %u:%u %u:%u\n",
+ n += sysfs_emit_at(data, n, "%s %u:%u %u:%u\n",
pd->disk->disk_name,
MAJOR(pd->pkt_dev), MINOR(pd->pkt_dev),
MAJOR(pd->bdev->bd_dev),
--
2.39.0

2023-01-19 22:29:31

by Andy Shevchenko

Subject: [PATCH v1 8/9] pktcdvd: Get rid of redundant 'else'

In the snippets like the following

if (...)
return / goto / break / continue ...;
else
...

the 'else' is redundant. Get rid of it.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/block/pktcdvd.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 340e953e3888..22048739a245 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -958,25 +958,25 @@ static int pkt_set_segment_merging(struct pktcdvd_device *pd, struct request_que
{
struct device *ddev = disk_to_dev(pd->disk);

- if ((pd->settings.size << 9) / CD_FRAMESIZE
- <= queue_max_segments(q)) {
+ if ((pd->settings.size << 9) / CD_FRAMESIZE <= queue_max_segments(q)) {
/*
* The cdrom device can handle one segment/frame
*/
clear_bit(PACKET_MERGE_SEGS, &pd->flags);
return 0;
- } else if ((pd->settings.size << 9) / PAGE_SIZE
- <= queue_max_segments(q)) {
+ }
+
+ if ((pd->settings.size << 9) / PAGE_SIZE <= queue_max_segments(q)) {
/*
* We can handle this case at the expense of some extra memory
* copies during write operations
*/
set_bit(PACKET_MERGE_SEGS, &pd->flags);
return 0;
- } else {
- dev_err(ddev, "cdrom max_phys_segments too small\n");
- return -EIO;
}
+
+ dev_err(ddev, "cdrom max_phys_segments too small\n");
+ return -EIO;
}

static void pkt_end_io_read(struct bio *bio)
--
2.39.0

2023-01-19 22:29:45

by Andy Shevchenko

Subject: [PATCH v1 5/9] pktcdvd: Drop redundant castings for sector_t

Since the commit 72deb455b5ec ("block: remove CONFIG_LBDAF")
the sector_t is always 64-bit type, no need to cast anymore.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/block/pktcdvd.c | 26 ++++++++++----------------
1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 96db6e348e1e..a53d21dbf2ab 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -493,7 +493,7 @@ static int pkt_seq_show(struct seq_file *m, void *p)
seq_printf(m, "\nQueue state:\n");
seq_printf(m, "\tbios queued:\t\t%d\n", pd->bio_queue_size);
seq_printf(m, "\tbios pending:\t\t%d\n", atomic_read(&pd->cdrw.pending_bios));
- seq_printf(m, "\tcurrent sector:\t\t0x%llx\n", (unsigned long long)pd->current_sector);
+ seq_printf(m, "\tcurrent sector:\t\t0x%llx\n", pd->current_sector);

pkt_count_states(pd, states);
seq_printf(m, "\tstate:\t\t\ti:%d ow:%d rw:%d ww:%d rec:%d fin:%d\n",
@@ -1003,8 +1003,7 @@ static void pkt_end_io_read(struct bio *bio)
BUG_ON(!pd);

dev_dbg(disk_to_dev(pd->disk), "bio=%p sec0=%llx sec=%llx err=%d\n",
- bio, (unsigned long long)pkt->sector,
- (unsigned long long)bio->bi_iter.bi_sector, bio->bi_status);
+ bio, pkt->sector, bio->bi_iter.bi_sector, bio->bi_status);

if (bio->bi_status)
atomic_inc(&pkt->io_errors);
@@ -1067,7 +1066,7 @@ static void pkt_gather_data(struct pktcdvd_device *pd, struct packet_data *pkt)
spin_unlock(&pkt->lock);

if (pkt->cache_valid) {
- dev_dbg(ddev, "zone %llx cached\n", (unsigned long long)pkt->sector);
+ dev_dbg(ddev, "zone %llx cached\n", pkt->sector);
goto out_account;
}

@@ -1099,8 +1098,7 @@ static void pkt_gather_data(struct pktcdvd_device *pd, struct packet_data *pkt)
}

out_account:
- dev_dbg(ddev, "need %d frames for zone %llx\n", frames_read,
- (unsigned long long)pkt->sector);
+ dev_dbg(ddev, "need %d frames for zone %llx\n", frames_read, pkt->sector);
pd->stats.pkt_started++;
pd->stats.secs_rg += frames_read * (CD_FRAMESIZE >> 9);
}
@@ -1143,8 +1141,7 @@ static inline void pkt_set_state(struct device *ddev, struct packet_data *pkt,
enum packet_data_state old_state = pkt->state;

dev_dbg(ddev, "pkt %2d : s=%6llx %s -> %s\n",
- pkt->id, (unsigned long long)pkt->sector,
- state_name[old_state], state_name[state]);
+ pkt->id, pkt->sector, state_name[old_state], state_name[state]);

pkt->state = state;
}
@@ -1218,12 +1215,12 @@ static int pkt_handle_queue(struct pktcdvd_device *pd)
* to this packet.
*/
spin_lock(&pd->lock);
- dev_dbg(ddev, "looking for zone %llx\n", (unsigned long long)zone);
+ dev_dbg(ddev, "looking for zone %llx\n", zone);
while ((node = pkt_rbtree_find(pd, zone)) != NULL) {
sector_t tmp = get_zone(node->bio->bi_iter.bi_sector, pd);

bio = node->bio;
- dev_dbg(ddev, "found zone=%llx\n", (unsigned long long)tmp);
+ dev_dbg(ddev, "found zone=%llx\n", tmp);
if (tmp != zone)
break;
pkt_rbtree_erase(pd, node);
@@ -1323,8 +1320,7 @@ static void pkt_start_write(struct pktcdvd_device *pd, struct packet_data *pkt)
pkt_set_state(ddev, pkt, PACKET_WRITE_WAIT_STATE);
spin_unlock(&pkt->lock);

- dev_dbg(ddev, "Writing %d frames for zone %llx\n", pkt->write_size,
- (unsigned long long)pkt->sector);
+ dev_dbg(ddev, "Writing %d frames for zone %llx\n", pkt->write_size, pkt->sector);

if (test_bit(PACKET_MERGE_SEGS, &pd->flags) || (pkt->write_size < pkt->frames))
pkt->cache_valid = 1;
@@ -2472,8 +2468,7 @@ static void pkt_submit_bio(struct bio *bio)
return;

dev_dbg(ddev, "start = %6llx stop = %6llx\n",
- (unsigned long long)bio->bi_iter.bi_sector,
- (unsigned long long)bio_end_sector(bio));
+ bio->bi_iter.bi_sector, bio_end_sector(bio));

/*
* Clone READ bios so we can have our own bi_end_io callback.
@@ -2484,8 +2479,7 @@ static void pkt_submit_bio(struct bio *bio)
}

if (!test_bit(PACKET_WRITABLE, &pd->flags)) {
- dev_notice(ddev, "WRITE for ro device (%llu)\n",
- (unsigned long long)bio->bi_iter.bi_sector);
+ dev_notice(ddev, "WRITE for ro device (%llu)\n", bio->bi_iter.bi_sector);
goto end_io;
}

--
2.39.0

2023-01-19 22:55:11

by Andy Shevchenko

Subject: [PATCH v1 7/9] pktcdvd: Use put_unaligned_be16() and get_unaligned_be16()

This makes the driver code slightly better to understand.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/block/pktcdvd.c | 31 ++++++++++++++-----------------
1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 2c5441303e55..340e953e3888 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -70,6 +70,8 @@
#include <linux/nospec.h>
#include <linux/uaccess.h>

+#include <asm/unaligned.h>
+
#define DRIVER_NAME "pktcdvd"

#define MAX_SPEED 0xffff
@@ -825,10 +827,8 @@ static noinline_for_stack int pkt_set_speed(struct pktcdvd_device *pd,
init_cdrom_command(&cgc, NULL, 0, CGC_DATA_NONE);
cgc.sshdr = &sshdr;
cgc.cmd[0] = GPCMD_SET_SPEED;
- cgc.cmd[2] = (read_speed >> 8) & 0xff;
- cgc.cmd[3] = read_speed & 0xff;
- cgc.cmd[4] = (write_speed >> 8) & 0xff;
- cgc.cmd[5] = write_speed & 0xff;
+ put_unaligned_be16(read_speed, &cgc.cmd[2]);
+ put_unaligned_be16(write_speed, &cgc.cmd[4]);

ret = pkt_generic_packet(pd, &cgc);
if (ret)
@@ -1526,8 +1526,7 @@ static int pkt_mode_sense(struct pktcdvd_device *pd, struct packet_command *cgc,

cgc->cmd[0] = GPCMD_MODE_SENSE_10;
cgc->cmd[2] = page_code | (page_control << 6);
- cgc->cmd[7] = cgc->buflen >> 8;
- cgc->cmd[8] = cgc->buflen & 0xff;
+ put_unaligned_be16(cgc->buflen, &cgc->cmd[7]);
cgc->data_direction = CGC_DATA_READ;
return pkt_generic_packet(pd, cgc);
}
@@ -1538,8 +1537,7 @@ static int pkt_mode_select(struct pktcdvd_device *pd, struct packet_command *cgc
memset(cgc->buffer, 0, 2);
cgc->cmd[0] = GPCMD_MODE_SELECT_10;
cgc->cmd[1] = 0x10; /* PF */
- cgc->cmd[7] = cgc->buflen >> 8;
- cgc->cmd[8] = cgc->buflen & 0xff;
+ put_unaligned_be16(cgc->buflen, &cgc->cmd[7]);
cgc->data_direction = CGC_DATA_WRITE;
return pkt_generic_packet(pd, cgc);
}
@@ -1580,8 +1578,7 @@ static int pkt_get_track_info(struct pktcdvd_device *pd, __u16 track, __u8 type,
init_cdrom_command(&cgc, ti, 8, CGC_DATA_READ);
cgc.cmd[0] = GPCMD_READ_TRACK_RZONE_INFO;
cgc.cmd[1] = type & 3;
- cgc.cmd[4] = (track & 0xff00) >> 8;
- cgc.cmd[5] = track & 0xff;
+ put_unaligned_be16(track, &cgc.cmd[4]);
cgc.cmd[8] = 8;
cgc.quiet = 1;

@@ -1662,8 +1659,8 @@ static noinline_for_stack int pkt_set_write_settings(struct pktcdvd_device *pd)
return ret;
}

- size = 2 + ((buffer[0] << 8) | (buffer[1] & 0xff));
- pd->mode_offset = (buffer[6] << 8) | (buffer[7] & 0xff);
+ size = 2 + get_unaligned_be16(&buffer[0]);
+ pd->mode_offset = get_unaligned_be16(&buffer[6]);
if (size > sizeof(buffer))
size = sizeof(buffer);

@@ -1820,7 +1817,7 @@ static noinline_for_stack int pkt_probe_settings(struct pktcdvd_device *pd)
cgc.cmd[0] = GPCMD_GET_CONFIGURATION;
cgc.cmd[8] = 8;
ret = pkt_generic_packet(pd, &cgc);
- pd->mmc3_profile = ret ? 0xffff : buf[6] << 8 | buf[7];
+ pd->mmc3_profile = ret ? 0xffff : get_unaligned_be16(&buf[6]);

memset(&di, 0, sizeof(disc_information));
memset(&ti, 0, sizeof(track_information));
@@ -1933,7 +1930,7 @@ static noinline_for_stack int pkt_write_caching(struct pktcdvd_device *pd,

buf[pd->mode_offset + 10] |= (!!set << 2);

- cgc.buflen = cgc.cmd[8] = 2 + ((buf[0] << 8) | (buf[1] & 0xff));
+ cgc.buflen = cgc.cmd[8] = 2 + get_unaligned_be16(&buf[0]);
ret = pkt_mode_select(pd, &cgc);
if (ret) {
dev_err(ddev, "write caching control failed\n");
@@ -1988,12 +1985,12 @@ static noinline_for_stack int pkt_get_max_speed(struct pktcdvd_device *pd,
* Speed Performance Descriptor Block", use the information
* in the first block. (contains the highest speed)
*/
- int num_spdb = (cap_buf[30] << 8) + cap_buf[31];
+ int num_spdb = get_unaligned_be16(&cap_buf[30]);
if (num_spdb > 0)
offset = 34;
}

- *write_speed = (cap_buf[offset] << 8) | cap_buf[offset + 1];
+ *write_speed = get_unaligned_be16(&cap_buf[offset]);
return 0;
}

@@ -2038,7 +2035,7 @@ static noinline_for_stack int pkt_media_speed(struct pktcdvd_device *pd,
pkt_dump_sense(pd, &cgc);
return ret;
}
- size = ((unsigned int) buf[0]<<8) + buf[1] + 2;
+ size = 2 + get_unaligned_be16(&buf[0]);
if (size > sizeof(buf))
size = sizeof(buf);

--
2.39.0

2023-01-19 23:17:33

by Andy Shevchenko

Subject: [PATCH v1 2/9] pktcdvd: replace sscanf() by kstrtoul()

The checkpatch.pl warns: "Prefer kstrto<type> to single variable sscanf".
Fix the code accordingly.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/block/pktcdvd.c | 34 ++++++++++++++++++----------------
1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 0ec8dc8ee5ed..ad4336ae9927 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -236,15 +236,16 @@ static ssize_t congestion_off_store(struct device *dev,
const char *buf, size_t len)
{
struct pktcdvd_device *pd = dev_get_drvdata(dev);
- int val;
+ int val, ret;

- if (sscanf(buf, "%d", &val) == 1) {
- spin_lock(&pd->lock);
- pd->write_congestion_off = val;
- init_write_congestion_marks(&pd->write_congestion_off,
- &pd->write_congestion_on);
- spin_unlock(&pd->lock);
- }
+ ret = kstrtoint(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ spin_lock(&pd->lock);
+ pd->write_congestion_off = val;
+ init_write_congestion_marks(&pd->write_congestion_off, &pd->write_congestion_on);
+ spin_unlock(&pd->lock);
return len;
}
static DEVICE_ATTR_RW(congestion_off);
@@ -266,15 +267,16 @@ static ssize_t congestion_on_store(struct device *dev,
const char *buf, size_t len)
{
struct pktcdvd_device *pd = dev_get_drvdata(dev);
- int val;
+ int val, ret;

- if (sscanf(buf, "%d", &val) == 1) {
- spin_lock(&pd->lock);
- pd->write_congestion_on = val;
- init_write_congestion_marks(&pd->write_congestion_off,
- &pd->write_congestion_on);
- spin_unlock(&pd->lock);
- }
+ ret = kstrtoint(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ spin_lock(&pd->lock);
+ pd->write_congestion_on = val;
+ init_write_congestion_marks(&pd->write_congestion_off, &pd->write_congestion_on);
+ spin_unlock(&pd->lock);
return len;
}
static DEVICE_ATTR_RW(congestion_on);
--
2.39.0

2023-01-20 07:37:20

by Greg Kroah-Hartman

Subject: Re: [PATCH v1 6/9] pktcdvd: Use DEFINE_SHOW_ATTRIBUTE() to simplify code

On Fri, Jan 20, 2023 at 12:08:06AM +0200, Andy Shevchenko wrote:
> Use DEFINE_SHOW_ATTRIBUTE() helper macro to simplify the code.
> No functional change.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> drivers/block/pktcdvd.c | 23 +++--------------------
> 1 file changed, 3 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
> index a53d21dbf2ab..2c5441303e55 100644
> --- a/drivers/block/pktcdvd.c
> +++ b/drivers/block/pktcdvd.c
> @@ -504,24 +504,7 @@ static int pkt_seq_show(struct seq_file *m, void *p)
> pd->write_congestion_on);
> return 0;
> }
> -
> -static int pkt_debugfs_seq_show(struct seq_file *m, void *p)
> -{
> - return pkt_seq_show(m, p);
> -}
> -
> -static int pkt_debugfs_fops_open(struct inode *inode, struct file *file)
> -{
> - return single_open(file, pkt_debugfs_seq_show, inode->i_private);
> -}
> -
> -static const struct file_operations debug_fops = {
> - .open = pkt_debugfs_fops_open,
> - .read = seq_read,
> - .llseek = seq_lseek,
> - .release = single_release,
> - .owner = THIS_MODULE,
> -};
> +DEFINE_SHOW_ATTRIBUTE(pkt_seq);
>
> static void pkt_debugfs_dev_new(struct pktcdvd_device *pd)
> {
> @@ -531,8 +514,8 @@ static void pkt_debugfs_dev_new(struct pktcdvd_device *pd)
> if (!pd->dfs_d_root)
> return;

This check shouldn't be here either.

> - pd->dfs_f_info = debugfs_create_file("info", 0444,
> - pd->dfs_d_root, pd, &debug_fops);
> + pd->dfs_f_info = debugfs_create_file("info", 0444, pd->dfs_d_root,
> + pd, &pkt_seq_fops);

For further cleanup, there's no need to keep this dentry at all as the
whole directory is later removed.

But that's future cleanups, for this, nice work:

Reviewed-by: Greg Kroah-Hartman <[email protected]>

2023-01-20 07:38:08

by Greg Kroah-Hartman

Subject: Re: [PATCH v1 8/9] pktcdvd: Get rid of redundant 'else'

On Fri, Jan 20, 2023 at 12:08:08AM +0200, Andy Shevchenko wrote:
> In the snippets like the following
>
> if (...)
> return / goto / break / continue ...;
> else
> ...
>
> the 'else' is redundant. Get rid of it.
>
> Signed-off-by: Andy Shevchenko <[email protected]>

Reviewed-by: Greg Kroah-Hartman <[email protected]>

2023-01-20 07:39:38

by Greg Kroah-Hartman

Subject: Re: [PATCH v1 3/9] pktcdvd: use sysfs_emit() to instead of scnprintf()

On Fri, Jan 20, 2023 at 12:08:03AM +0200, Andy Shevchenko wrote:
> Follow the advice of the Documentation/filesystems/sysfs.rst and show()
> should only use sysfs_emit() or sysfs_emit_at() when formatting the
> value to be returned to user space.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> drivers/block/pktcdvd.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>

Reviewed-by: Greg Kroah-Hartman <[email protected]>

2023-01-20 08:00:27

by Greg Kroah-Hartman

Subject: Re: [PATCH v1 5/9] pktcdvd: Drop redundant castings for sector_t

On Fri, Jan 20, 2023 at 12:08:05AM +0200, Andy Shevchenko wrote:
> Since the commit 72deb455b5ec ("block: remove CONFIG_LBDAF")
> the sector_t is always 64-bit type, no need to cast anymore.
>
> Signed-off-by: Andy Shevchenko <[email protected]>

Reviewed-by: Greg Kroah-Hartman <[email protected]>

2023-01-20 08:28:00

by Greg Kroah-Hartman

Subject: Re: [PATCH v1 1/9] pktcdvd: Get rid of custom printing macros

On Fri, Jan 20, 2023 at 12:08:01AM +0200, Andy Shevchenko wrote:
> We may use traditional dev_*() macros instead of custom ones
> provided by the driver.
>
> Signed-off-by: Andy Shevchenko <[email protected]>

Reviewed-by: Greg Kroah-Hartman <[email protected]>