[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [tyndur-devel] [PATCH] cdi: CD-ROM-Unterstuetzung

Date: Sat, 3 Jan 2009 22:44:37 +0100
From: Kevin Wolf <mail@xxxxxxxxxxxxx>
To: "tyndur-Patches" <tyndur-devel@xxxxxxxxxx>
Subject: Re: [tyndur-devel] [PATCH] cdi: CD-ROM-Unterstuetzung
Message-id: <200901032244.37511@kevin-wolf.de>
In-reply-to: <20090103205246.GA31201@thales.produktiv.lan>
Am Samstag, 3. Januar 2009 21:52:46 schrieb Antoine Kaufmann:
> On Sat, Jan 03 21:19, Kevin Wolf wrote:
> > + cdi: Storage: Zusaetzliche Funktionen get_size() und get_block_size(),
> > da SCSI-Geraete die Variablen nicht haben + cdi: SCSI:
> > Grundfunktionalitaet
> > + cdi: SCSI: Storage-Backend scsi/disk (momentan nur fuer CD-ROM
> > geeignet) * cdi: Storage nimmt jetzt auch SCSI-Geraete
> > * cdi: Storage-Treiber und -Geraete werden einzeln initialisiert
> > * ata/floppy: An geaenderte Interfaces geanpasst
> > ---
> >  src/modules/cdi/ata/atapi.c           |    3 +
> >  src/modules/cdi/ata/device.c          |   19 ++-
> >  src/modules/cdi/ata/device.h          |    6 +-
> >  src/modules/cdi/ata/main.c            |    4 +-
> >  src/modules/cdi/floppy/device.c       |   29 +++-
> >  src/modules/cdi/floppy/device.h       |    6 +-
> >  src/modules/cdi/floppy/main.c         |    8 +-
> >  src/modules/cdi/include/cdi.h         |    3 +-
> >  src/modules/cdi/include/cdi/scsi.h    |   86 +++++++++++
> >  src/modules/cdi/include/cdi/storage.h |   19 ++-
> >  src/modules/cdi/lib/cdi.c             |    1 +
> >  src/modules/cdi/lib/scsi/disk.c       |  264
> > +++++++++++++++++++++++++++++++++ src/modules/cdi/lib/scsi/driver.c     |
> >   56 +++++++
> >  src/modules/cdi/lib/storage.c         |   89 ++++++++----
> >  14 files changed, 546 insertions(+), 47 deletions(-)
> >  create mode 100644 src/modules/cdi/include/cdi/scsi.h
> >  create mode 100644 src/modules/cdi/lib/scsi/disk.c
> >  create mode 100644 src/modules/cdi/lib/scsi/driver.c
> >
> > diff --git a/src/modules/cdi/ata/atapi.c b/src/modules/cdi/ata/atapi.c
> > index 67dc140..ecff315 100644
> > --- a/src/modules/cdi/ata/atapi.c
> > +++ b/src/modules/cdi/ata/atapi.c
> > @@ -86,7 +86,10 @@ int atapi_drv_identify(struct ata_device* dev)
> >
> >  void atapi_init_device(struct cdi_device* device)
> >  {
> > +    struct cdi_scsi_device* scsi = (struct cdi_scsi_device*) device;
> >
> > +    scsi->type = CDI_STORAGE;
> > +    cdi_scsi_device_init(scsi);
> >  }
> >
> >  void atapi_remove_device(struct cdi_device* device)
> > diff --git a/src/modules/cdi/ata/device.c b/src/modules/cdi/ata/device.c
> > index f9ccf21..b9763d4 100644
> > --- a/src/modules/cdi/ata/device.c
> > +++ b/src/modules/cdi/ata/device.c
> > @@ -333,6 +333,8 @@ void ata_remove_controller(struct ata_controller*
> > controller)
> >
> >  void ata_init_device(struct cdi_device* device)
> >  {
> > +    struct ata_device* dev = (struct ata_device*) device;
> > +    cdi_storage_device_init(&dev->dev.storage);
> >  }
> >
> >  void ata_remove_device(struct cdi_device* device)
> > @@ -344,7 +346,7 @@ void ata_remove_device(struct cdi_device* device)
> >  /**
> >   * Blocks von einem ATA(PI) Geraet lesen
> >   */
> > -int ata_read_blocks(struct cdi_storage_device* device, uint64_t block,
> > +int ata_read_blocks(struct cdi_device* device, uint64_t block,
> >      uint64_t count, void* buffer)
> >  {
> >      struct ata_device* dev = (struct ata_device*) device;
> > @@ -374,7 +376,7 @@ int ata_read_blocks(struct cdi_storage_device*
> > device, uint64_t block, /**
> >   * Blocks auf ein ATA(PI) Geraet schreiben
> >   */
> > -int ata_write_blocks(struct cdi_storage_device* device, uint64_t block,
> > +int ata_write_blocks(struct cdi_device* device, uint64_t block,
> >      uint64_t count, void* buffer)
> >  {
> >      struct ata_device* dev = (struct ata_device*) device;
> > @@ -401,3 +403,16 @@ int ata_write_blocks(struct cdi_storage_device*
> > device, uint64_t block, }
> >  }
> >
> > +uint64_t ata_get_size(struct cdi_device* device)
> > +{
> > +    struct cdi_storage_device* storage = (struct cdi_storage_device*)
> > device; +
> > +    return storage->block_size * storage->block_count;
> > +}
> > +
> > +uint64_t ata_get_block_size(struct cdi_device* device)
> > +{
> > +    struct cdi_storage_device* storage = (struct cdi_storage_device*)
> > device; +
> > +    return storage->block_size;
> > +}
> > diff --git a/src/modules/cdi/ata/device.h b/src/modules/cdi/ata/device.h
> > index 140f037..eaa890a 100644
> > --- a/src/modules/cdi/ata/device.h
> > +++ b/src/modules/cdi/ata/device.h
> > @@ -411,11 +411,13 @@ void ata_init_controller(struct ata_controller*
> > controller); void ata_remove_controller(struct ata_controller*
> > controller);
> >  void ata_init_device(struct cdi_device* device);
> >  void ata_remove_device(struct cdi_device* device);
> > -int ata_read_blocks(struct cdi_storage_device* device, uint64_t block,
> > +int ata_read_blocks(struct cdi_device* device, uint64_t block,
> >      uint64_t count, void* buffer);
> > -int ata_write_blocks(struct cdi_storage_device* device, uint64_t block,
> > +int ata_write_blocks(struct cdi_device* device, uint64_t block,
> >      uint64_t count, void* buffer);
> >
> > +uint64_t ata_get_size(struct cdi_device* device);
> > +uint64_t ata_get_block_size(struct cdi_device* device);
> >
> >  // Einen ATA-Request absenden und ausfuehren
> >  int ata_request(struct ata_request* request);
> > diff --git a/src/modules/cdi/ata/main.c b/src/modules/cdi/ata/main.c
> > index 750f7b2..18e7225 100644
> > --- a/src/modules/cdi/ata/main.c
> > +++ b/src/modules/cdi/ata/main.c
> > @@ -80,7 +80,7 @@ static void ata_driver_init()
> >      // Konstruktor der Vaterklasse
> >      cdi_storage_driver_init((struct cdi_storage_driver*)
> > &driver_storage); cdi_scsi_driver_init((struct cdi_scsi_driver*)
> > &driver_scsi);
>
> Hm sicher dass dein diff hier korrekt ist? Haben wir da wirklich schon
> ein cdi_scsi_driver_init drin? :-/

Da drunter ist noch janoschs ATAPI-Patch, zu dem es ja keine Kommentare mehr 
gab. Der fügt das ein.

> > -
> > +
>
> Und hier haben wir noch eine Whitespace Änderung, sowas aber auch. ;-)

Bäh.

>
> >      // Namen setzen
> >      driver_storage.drv.name = driver_storage_name;
> >      driver_scsi.drv.name = driver_scsi_name;
> > @@ -91,6 +91,8 @@ static void ata_driver_init()
> >      driver_storage.drv.remove_device    = ata_remove_device;
> >      driver_storage.read_blocks          = ata_read_blocks;
> >      driver_storage.write_blocks         = ata_write_blocks;
> > +    driver_storage.get_size             = ata_get_size;
> > +    driver_storage.get_block_size       = ata_get_block_size;
> >
> >      driver_scsi.drv.destroy             = atapi_driver_destroy;
> >      driver_scsi.drv.init_device         = atapi_init_device;
> > diff --git a/src/modules/cdi/floppy/device.c
> > b/src/modules/cdi/floppy/device.c index 6614e1e..4435630 100644
> > --- a/src/modules/cdi/floppy/device.c
> > +++ b/src/modules/cdi/floppy/device.c
> > @@ -796,6 +796,8 @@ void floppy_init_device(struct cdi_device* device)
> >
> >      dev->dev.block_size = FLOPPY_SECTOR_SIZE(dev);
> >      dev->dev.block_count = FLOPPY_SECTOR_COUNT(dev);
> > +
> > +    cdi_storage_device_init(&dev->dev);
> >  }
> >
> >  /**
> > @@ -808,7 +810,7 @@ void floppy_remove_device(struct cdi_device* device)
> >  /**
> >   * Sektoren einlesen
> >   */
> > -int floppy_read_blocks(struct cdi_storage_device* dev, uint64_t block,
> > +int floppy_read_blocks(struct cdi_device* dev, uint64_t block,
> >      uint64_t count, void* buffer)
> >  {
> >      struct floppy_device* device = (struct floppy_device*) dev;
> > @@ -823,7 +825,7 @@ int floppy_read_blocks(struct cdi_storage_device*
> > dev, uint64_t block, // Wenn das schief geht, wird mehrmals probiert
> >          for (j = 0; (j < 5) && (result != 0); j++) {
> >              result = floppy_drive_sector_read(device, (uint32_t) block +
> > i, -                buffer + i * dev->block_size);
> > +                buffer + i * device->dev.block_size);
> >          }
> >      }
> >
> > @@ -833,7 +835,7 @@ int floppy_read_blocks(struct cdi_storage_device*
> > dev, uint64_t block, /**
> >   * Sektoren schreiben
> >   */
> > -int floppy_write_blocks(struct cdi_storage_device* dev, uint64_t block,
> > +int floppy_write_blocks(struct cdi_device* dev, uint64_t block,
> >      uint64_t count, void* buffer)
> >  {
> >      struct floppy_device* device = (struct floppy_device*) dev;
> > @@ -848,13 +850,32 @@ int floppy_write_blocks(struct cdi_storage_device*
> > dev, uint64_t block, // Wenn das schief geht, wird mehrmals probiert
> >          for (j = 0; (j < 5) && (result != 0); j++) {
> >              result = floppy_drive_sector_write(device, (uint32_t) block
> > + i, -                buffer + i * dev->block_size);
> > +                buffer + i * device->dev.block_size);
> >          }
> >      }
> >
> >      return result;
> >  }
> >
> > +/**
> > + * Gibt die Groesse des Mediums zurueck
> > + */
> > +uint64_t floppy_get_size(struct cdi_device* device)
> > +{
> > +    struct cdi_storage_device* storage = (struct cdi_storage_device*)
> > device; +
> > +    return storage->block_size * storage->block_count;
> > +}
> > +
> > +/**
> > + * Gibt die Sektorgroesse des Mediums zurueck
> > + */
> > +uint64_t floppy_get_block_size(struct cdi_device* device)
> > +{
> > +    struct cdi_storage_device* storage = (struct cdi_storage_device*)
> > device; +
> > +    return storage->block_size;
> > +}
> >
> >  /**
> >   * IRQ-Handler fuer den Kontroller; Erhoeht nur den IRQ-Zaehler in der
> > diff --git a/src/modules/cdi/floppy/device.h
> > b/src/modules/cdi/floppy/device.h index 24d16f9..6c3bfcf 100644
> > --- a/src/modules/cdi/floppy/device.h
> > +++ b/src/modules/cdi/floppy/device.h
> > @@ -137,10 +137,12 @@ int floppy_init_controller(struct
> > floppy_controller* controller);
> >
> >  void floppy_init_device(struct cdi_device* device);
> >  void floppy_remove_device(struct cdi_device* device);
> > -int floppy_read_blocks(struct cdi_storage_device* device, uint64_t
> > block, +int floppy_read_blocks(struct cdi_device* device, uint64_t block,
> > uint64_t count, void* buffer);
> > -int floppy_write_blocks(struct cdi_storage_device* device, uint64_t
> > block, +int floppy_write_blocks(struct cdi_device* device, uint64_t
> > block, uint64_t count, void* buffer);
> > +uint64_t floppy_get_size(struct cdi_device* device);
> > +uint64_t floppy_get_block_size(struct cdi_device* device);
> >
> >  void floppy_handle_interrupt(struct cdi_device* device);
> >
> > diff --git a/src/modules/cdi/floppy/main.c
> > b/src/modules/cdi/floppy/main.c index 435ff1f..aa32ed5 100644
> > --- a/src/modules/cdi/floppy/main.c
> > +++ b/src/modules/cdi/floppy/main.c
> > @@ -87,7 +87,7 @@ static int floppy_driver_init(struct floppy_driver*
> > driver)
> >
> >      // Konstruktor der Vaterklasse
> >      cdi_storage_driver_init((struct cdi_storage_driver*) driver);
> > -
> > +
>
> Schon wieder nur leerzeichen entfernt
>
> >      // Namen setzen
> >      driver->storage.drv.name = driver_name;
> >
> > @@ -95,9 +95,11 @@ static int floppy_driver_init(struct floppy_driver*
> > driver) driver->storage.drv.destroy         = floppy_driver_destroy;
> > driver->storage.drv.init_device     = floppy_init_device;
> >      driver->storage.drv.remove_device   = floppy_remove_device;
> > -    driver->storage.read_blocks         = floppy_read_blocks;
> > +    driver->storage.read_blocks         = floppy_read_blocks;
>
> Hier auch wieder
>
> >      driver->storage.write_blocks        = floppy_write_blocks;
> > -
> > +    driver->storage.get_size            = floppy_get_size;
> > +    driver->storage.get_block_size      = floppy_get_block_size;
> > +
> >      // Geraete erstellen (TODO: Was wenn eines oder beide nicht
> > vorhanden // sind?)
> >      for (i = 0; i < 2; i++) {
> > diff --git a/src/modules/cdi/include/cdi.h
> > b/src/modules/cdi/include/cdi.h index 42e4643..5fb7025 100644
> > --- a/src/modules/cdi/include/cdi.h
> > +++ b/src/modules/cdi/include/cdi.h
> > @@ -23,7 +23,8 @@
> >  typedef enum {
> >      CDI_UNKNOWN         = 0,
> >      CDI_NETWORK         = 1,
> > -    CDI_STORAGE         = 2
> > +    CDI_STORAGE         = 2,
> > +    CDI_SCSI            = 3,
> >  } cdi_device_type_t;
> >
> >  struct cdi_driver;
> > diff --git a/src/modules/cdi/include/cdi/scsi.h
> > b/src/modules/cdi/include/cdi/scsi.h new file mode 100644
> > index 0000000..78f0c30
> > --- /dev/null
> > +++ b/src/modules/cdi/include/cdi/scsi.h
> > @@ -0,0 +1,86 @@
>
> Gibts hier keinen Lizenzheader?
>
> Dann könntest du auch noch gleich die Doxygen-Anweisungen reinpacken,
> damit das ins SCSI-Modul kommt.

Ok

>
> > +#ifndef _CDI_SCSI_H_
> > +#define _CDI_SCSI_H_
> > +
> > +#include <stdint.h>
> > +#include <stddef.h>
> > +
> > +#include "cdi.h"
> > +
> > +// SCSI-Paket
>
> Hier bitte alles so kommentieren dass Doxygen es versteht, also entweder
> /** blub */ oder /// blub.

Das ist doch alles gar nicht mein Code. Hach... ;-)

>
> > +struct cdi_scsi_packet {
> > +  // Buffer zum Senden oder Empfangen von Daten
> > +  void *buffer;
> > +
> > +  // Groesse des Buffers
> > +  size_t bufsize;
> > +
> > +  // Ob gelesen oder geschrieben werden soll
> > +  enum {
> > +    CDI_SCSI_NODATA,
> > +    CDI_SCSI_READ,
> > +    CDI_SCSI_WRITE,
> > +  } direction;
> > +
> > +  // SCSI Command
> > +  uint8_t command[16];
> > +
> > +  // Groesse des SCSI Commands
> > +  size_t cmdsize;
> > +};
> > +
> > +// SCSI-Geraet
> > +struct cdi_scsi_device {
> > +
> > +    struct cdi_device dev;
> > +
> > +    // Geraetetyp, der ueber SCSI angesteuert wird
> > +    cdi_device_type_t type;
> > +
> > +};
> > +
> > +// SCSI-Driver
> > +struct cdi_scsi_driver {
> > +  struct cdi_driver drv;
> > +
>
> Hier hätte ich eigentlich auch irgend einen kommentar erwartet.
>
> > +  int (*request)(struct cdi_scsi_device *device,struct cdi_scsi_packet
> > *packet); +};
> > +
> > +/**
> > + * Ein SCSI-Paket allozieren
> > + *
> > + * @param size Benoetigte Groesse
> > + *
> > + * @return Pointer auf das Paket oder NULL im Fehlerfall
> > + */
> > +struct cdi_scsi_packet* cdi_scsi_packet_alloc(size_t size);
> > +
> > +/**
> > + * Ein SCSI-Paket freigeben
> > + *
> > + * @param packet Pointer auf das Paket
> > + */
> > +void cdi_scsi_packet_free(struct cdi_scsi_packet* packet);
> > +
> > +/**
> > + * Initialisiert die Datenstrukturen fuer einen SCSI-Treiber
> > + */
> > +void cdi_scsi_driver_init(struct cdi_scsi_driver* driver);
> > +
> > +/**
> > + * Deinitialisiert die Datenstrukturen fuer einen SCSI-Treiber
> > + */
> > +void cdi_scsi_driver_destroy(struct cdi_scsi_driver* driver);
> > +
> > +/**
> > + * Registiert einen SCSI-Treiber
> > + */
> > +void cdi_scsi_driver_register(struct cdi_scsi_driver* driver);
> > +
> > +/**
> > + * Initialisiert ein neues SCSI-Geraet
> > + *
> > + * Der Typ der Geraetes muss bereits gesetzt sein
> > + */
> > +void cdi_scsi_device_init(struct cdi_scsi_device* device);
> > +
> > +#endif
> > diff --git a/src/modules/cdi/include/cdi/storage.h
> > b/src/modules/cdi/include/cdi/storage.h index 73ae245..781a158 100644
> > --- a/src/modules/cdi/include/cdi/storage.h
> > +++ b/src/modules/cdi/include/cdi/storage.h
> > @@ -40,7 +40,7 @@ struct cdi_storage_driver {
> >       *
> >       * @return 0 bei Erfolg, -1 im Fehlerfall.
> >       */
> > -    int (*read_blocks)(struct cdi_storage_device* device, uint64_t
> > start, +    int (*read_blocks)(struct cdi_device* device, uint64_t start,
> > uint64_t count, void* buffer);
> >
> >      /**
> > @@ -53,8 +53,18 @@ struct cdi_storage_driver {
> >       *
> >       * @return 0 bei Erfolg, -1 im Fehlerfall
> >       */
> > -    int (*write_blocks)(struct cdi_storage_device* device, uint64_t
> > start, +    int (*write_blocks)(struct cdi_device* device, uint64_t
> > start, uint64_t count, void* buffer);
> > +
> > +    /**
> > +     * Gibt die Groesse des Mediums in Bytes zurueck
> > +     */
> > +    uint64_t (*get_size)(struct cdi_device* device);
> > +
> > +    /**
> > +     * Gibt die Blockgroesse des Geraets in Bytes zurueck
> > +     */
> > +    uint64_t (*get_block_size)(struct cdi_device* device);
> >  };
> >
> >
> > @@ -75,6 +85,11 @@ void cdi_storage_driver_destroy(struct
> > cdi_storage_driver* driver); */
> >  void cdi_storage_driver_register(struct cdi_storage_driver* driver);
> >
> > +/**
> > + * Initialisiert einen Massenspeicher
> > + */
> > +void cdi_storage_device_init(struct cdi_storage_device* device);
> > +
> >  #endif
> >
> >  /*\@}*/
> > diff --git a/src/modules/cdi/lib/cdi.c b/src/modules/cdi/lib/cdi.c
> > index d8c7819..487887e 100644
> > --- a/src/modules/cdi/lib/cdi.c
> > +++ b/src/modules/cdi/lib/cdi.c
> > @@ -86,6 +86,7 @@ void cdi_run_drivers(void)
> >
> >          for (j = 0; (device = cdi_list_get(driver->devices, j)); j++) {
> >              device->driver = driver;
> > +            device->type = driver->type;
> >
> >              if (driver->init_device) {
> >                  driver->init_device(device);
> > diff --git a/src/modules/cdi/lib/scsi/disk.c
> > b/src/modules/cdi/lib/scsi/disk.c new file mode 100644
> > index 0000000..9bd5a74
> > --- /dev/null
> > +++ b/src/modules/cdi/lib/scsi/disk.c
> > @@ -0,0 +1,264 @@
> > +/*
> > + * Copyright (C) 2008 Mathias Gottschlag
> > + * Copyright (C) 2009 Kevin Wolf
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining
> > a copy + * of this software and associated documentation files (the
> > "Software"), to + * deal in the Software without restriction, including
> > without limitation the + * rights to use, copy, modify, merge, publish,
> > distribute, sublicense, and/or + * sell copies of the Software, and to
> > permit persons to whom the Software is + * furnished to do so, subject to
> > the following conditions:
> > + *
> > + * The above copyright notice and this permission notice shall be
> > included in + * all copies or substantial portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND
> > NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS
> > BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN
> > ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN
> > CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE
> > SOFTWARE.
> > + */
>
> Hm müsste da nicht noch ein Hinweis in doc/COPYRIGHT?

Gute Frage. Ich pack es mal dazu rein.
References:
- [tyndur-devel] [PATCH] cdi: CD-ROM-Unterstuetzung
  - From: Kevin Wolf
- Re: [tyndur-devel] [PATCH] cdi: CD-ROM-Unterstuetzung
  - From: Antoine Kaufmann
Prev by Date: Re: [tyndur-devel] [PATCH] cdi: CD-ROM-Unterstuetzung
Next by Date: [tyndur-devel] [PATCH v2] cdi: CD-ROM-Unterstuetzung
Previous by thread: Re: [tyndur-devel] [PATCH] cdi: CD-ROM-Unterstuetzung
Next by thread: [tyndur-devel] [PATCH v2] cdi: CD-ROM-Unterstuetzung
Index(es):
- Date
- Thread